diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 987ceae78..6a6003b93 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Doctrine entities and repositories link to each other.** In a Doctrine project, a CodeLens on an entity opens its repository, and one on a repository opens its entity, found through the YAML or XML mapping or the usual naming convention. Contributed by @sidux. - **Symfony service IDs and container parameters.** In a Symfony project, a service ID or parameter declared in YAML, XML, or a PHP configurator completes, jumps to its declaration, and lists its references wherever configuration or code names it, from `@app.mailer` and `%app.sender%` to `$container->get()` and `#[Autowire]`. An application's own `app.` name that nothing declares is reported. Contributed by @sidux. - **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. - **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. - **Symfony Twig templates.** In a Symfony project, a template name completes, jumps to its file, and lists its references wherever a controller, an email, or another template names it, from `render()` and `#[Template]` to `extends` and `include`. A template the application's `templates/` directory does not hold is reported, with a quick fix that creates it, unless the project has a Twig loader of its own that could serve it. Contributed by @sidux. diff --git a/docs/todo.md b/docs/todo.md index 8dc3e7964..d2b4e338b 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 | -| DR2 | [Service IDs from `*.services.yml`](todo/drupal.md#dr2-service-ids-from-servicesyml) | High | Medium | | DR5 | [Route names](todo/drupal.md#dr5-route-names) | Medium-High | 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 | @@ -128,7 +127,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 | -| DR2 | [Service IDs from `*.services.yml`](todo/drupal.md#dr2-service-ids-from-servicesyml) | High | Medium | | DR5 | [Route names](todo/drupal.md#dr5-route-names) | Medium-High | Medium | | DR6 | [Hooks](todo/drupal.md#dr6-hooks) | Medium-High | Medium | | DR10 | [Plugin IDs](todo/drupal.md#dr10-plugin-ids) | Medium | Medium | @@ -139,6 +137,7 @@ ships, remove every row that names it. | DR7 | [Render arrays, theme hooks, and libraries](todo/drupal.md#dr7-render-arrays-theme-hooks-and-libraries) | Medium | Medium-High | | DR4 | [Entity field access](todo/drupal.md#dr4-entity-field-access) (depends on DR3) | Medium-High | High | | 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 | | DR21 | [Return types core's docblocks leave wide](todo/drupal.md#dr21-return-types-cores-docblocks-leave-wide) | Medium | Low-Medium | ## Yii diff --git a/docs/todo/bugs.md b/docs/todo/bugs.md index d304996e7..3328acf0a 100644 --- a/docs/todo/bugs.md +++ b/docs/todo/bugs.md @@ -196,3 +196,45 @@ the body binds `new static()` to the declaring class instead of keeping it `static`, so a call through a subclass gives the parent. Declaring `@return static` on the method or on an interface it implements gives the right type. + +### B732. `#[\AllowDynamicProperties]` does not allow dynamic properties + +**Impact: Medium · Complexity: Low** + +```php +#[\AllowDynamicProperties] +class Extension {} + +function f(Extension $extension): int { + return $extension->sort; // Property 'sort' not found on class 'Extension' +} +``` + +A class marked `#[\AllowDynamicProperties]` (or one extending such a +class) takes any property, so reading or writing one it does not declare +is no unknown member. The attribute is only read by class-name +completion today. PHPStan's `ClassReflection::allowsDynamicProperties()` +walks the parent chain for the attribute. Drupal core's `Extension` is +the common case: `$module_data[$name]->sort` and `->requires` are +reported about thirty times on core once its services are typed. + +### B733. A Drupal service a `ServiceProvider` alters keeps the class its YAML names + +**Impact: Medium · Complexity: Medium** + +```php +// core.services.yml: language_manager: { class: Drupal\Core\Language\LanguageManager } +// language module: LanguageServiceProvider::alter() calls +// $container->getDefinition('language_manager')->setClass(ConfigurableLanguageManager::class) +\Drupal::service('language_manager')->getLanguageConfigOverride('fr', 'system.site'); +// Method 'getLanguageConfigOverride' not found on class 'Drupal\Core\Language\LanguageManager' +``` + +A module's `ServiceProvider` class can replace the class of a +service another module declares, and the service is then built from the +replacement whenever that module is enabled. `\Drupal::service()` and +`$container->get()` are typed by the YAML class alone, so a call that +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. diff --git a/docs/todo/drupal.md b/docs/todo/drupal.md index 5cde59efb..28316b353 100644 --- a/docs/todo/drupal.md +++ b/docs/todo/drupal.md @@ -1,10 +1,10 @@ # PHPantom — Drupal -Known gaps and missing features in PHPantom's Drupal support. Today -Drupal support is project discovery only: the web root is detected, and -core, contrib, and the Drupal file extensions (`.module`, `.install`, -`.theme`, …) are indexed past `.gitignore`. Nothing reads Drupal's own -declarations: services, entity types, fields, routes, or hooks. +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 are read. Nothing reads +Drupal's other declarations yet: entity types, fields, routes, or hooks. The requests in [#48](https://github.com/PHPantom-dev/phpantom_lsp/issues/48) are the @@ -28,46 +28,6 @@ within the same impact tier. --- -## DR2. Service IDs from `*.services.yml` - -**Impact: High · Complexity: Medium** - -```php -$manager = \Drupal::service('acme.order_manager'); - -public static function create(ContainerInterface $container): static { - return new static($container->get('entity_type.manager')); -} -``` - -A module declares its services in `.services.yml`, in Symfony's -service format. Type `\Drupal::service()` and `$container->get()` by the -class the declaration names, and complete and navigate the service IDs, -including `@service_id` arguments in the YAML. Newer code registers a -service under its interface and fetches it with -`\Drupal::service(OrderManagerInterface::class)`; that ID is a class -name and types as that class without any lookup. - -The Symfony container reading already parses this format and follows -aliases and parameter placeholders. Start by checking what it picks up from a -Drupal module unchanged, then teach it where Drupal keeps the files -(every enabled module's and core's `*.services.yml`) instead of -`config/services.yaml`. - -Most of Drupal's navigation depends on this. On Drupal core, Find -References on `EntityStorageInterface::load()` misses about 240 calls -of the shape -`$this->container->get('entity_type.manager')->getStorage('node')->load(1)`. -They are the bulk of what a tool that types service lookups finds and -we don't. - -**Where to look:** `src/symfony/container.rs`, `src/symfony/resources.rs`, -PHPStan's Drupal extension (its service map), and PHPlumb -(`crates/phplumb-index/src/frameworks/drupal/services.rs`, MIT), which -reads the same files. - ---- - ## DR3. Entity storage and entity type IDs **Impact: High · Complexity: Medium-High** @@ -373,6 +333,22 @@ the event names complete where Drupal's event dispatcher takes one. --- +## DR17. Warn on unknown service IDs + +**Impact: Medium · Complexity: Low-Medium** + +```php +$manager = \Drupal::service('acme.order_manger'); +``` + +A service ID no `*.services.yml` declares is a typo or a module the site +lacks, and the container throws for it at run time. Report one where +`\Drupal::service()` and `$container->get()` take it. A module's +`ServiceProvider` class can register services in PHP, which no YAML file +lists, so an ID such a provider may register must not warn. + +--- + ## DR21. Return types core's docblocks leave wide **Impact: Medium · Complexity: Low-Medium** diff --git a/examples/drupal/README.md b/examples/drupal/README.md index 8306549f1..33a1baa32 100644 --- a/examples/drupal/README.md +++ b/examples/drupal/README.md @@ -21,6 +21,15 @@ Run `composer install` here first. Composer puts core and contrib into definition, so the chain keeps completing. - In `web/modules/custom/acme/acme.module`, complete after `$route_match->` inside the procedural `acme_help()` hook. +- Service IDs and container parameters navigate between the services files + and the code that looks them up, core's included. In + `web/modules/custom/acme/acme.module`, hover `$greeter`, typed by the class + `acme.services.yml` declares for `acme.greeter`, or go to the definition of + `'acme.greeter'`. In `web/modules/custom/acme/src/NoteCounter.php`, go to + the definition of `'entity_type.manager'`, which lands in core's + `core.services.yml`. Complete an ID inside `\Drupal::service('')`, + `$container->get('')`, or after `@` in `acme.services.yml`, and find the + references of a service from its declaration. ## What the Symfony support already reads diff --git a/examples/drupal/assertions.php b/examples/drupal/assertions.php index e7201b45b..3d84d4855 100644 --- a/examples/drupal/assertions.php +++ b/examples/drupal/assertions.php @@ -17,13 +17,16 @@ use Drupal\acme\Entity\Note; use Drupal\acme\Greeter; use Drupal\acme\Hook\AcmeHooks; +use Drupal\acme\NoteCounter; use Drupal\Component\FileCache\FileCacheFactory; use Drupal\Component\Serialization\Yaml; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\DependencyInjection\YamlFileLoader; use Drupal\Core\Entity\Attribute\ContentEntityType; use Drupal\Core\Entity\ContentEntityBase; +use Drupal\Core\Entity\EntityTypeManager; use Drupal\Core\Extension\InfoParser; use Drupal\Core\Hook\Attribute\Hook; @@ -91,6 +94,17 @@ function attributeArguments(ReflectionClass|ReflectionMethod $reflection, string check('The class acme.services.yml names exists', class_exists((string) $container->getDefinition('acme.greeter')->getClass())); check('The Greeter class name is an alias of acme.greeter', $container->hasAlias(Greeter::class) && (string) $container->getAlias(Greeter::class) === 'acme.greeter'); check("acme.greeter receives core's config.factory", (string) $container->getDefinition('acme.greeter')->getArgument(0) === 'config.factory'); +check('acme.greeter receives the acme.greeting_format parameter', $container->getDefinition('acme.greeter')->getArgument(1) === '%acme.greeting_format%'); +check('acme.greeting_format is a parameter declared in acme.services.yml', is_string($container->getParameter('acme.greeting_format'))); +check('Greeter takes the format as its second argument', (new ReflectionMethod(Greeter::class, '__construct'))->getParameters()[1]->getType()?->getName() === 'string'); + +// Core declares the services the module looks up by ID. +$core = new ContainerBuilder(); +(new YamlFileLoader($core))->load("$web/core/core.services.yml"); +check('core.services.yml declares config.factory', $core->hasDefinition('config.factory')); +check('entity_type.manager, which NoteCounter::create() looks up, is built from EntityTypeManager', $core->getDefinition('entity_type.manager')->getClass() === EntityTypeManager::class); +check('NoteCounter takes what entity_type.manager is', is_subclass_of(EntityTypeManager::class, (string) (new ReflectionMethod(NoteCounter::class, '__construct'))->getParameters()[0]->getType())); +check('NoteCounter is built from the container', is_subclass_of(NoteCounter::class, ContainerInjectionInterface::class)); // ─── Routes ───────────────────────────────────────────────────────────────── @@ -120,6 +134,8 @@ function attributeArguments(ReflectionClass|ReflectionMethod $reflection, string $themeHook = new ReflectionMethod(AcmeHooks::class, 'theme'); $themeHooks = (new AcmeHooks())->theme(); check('acme_help() implements hook_help procedurally', function_exists('acme_help')); +check('acme_help() also looks the greeter up by its class name, which acme.services.yml registers', str_contains((string) file_get_contents("$module/acme.module"), '\\Drupal::service(Greeter::class)') && $container->hasAlias(Greeter::class)); +check("acme_help() looks up acme.greeter, which acme.services.yml declares", str_contains((string) file_get_contents("$module/acme.module"), "\\Drupal::service('acme.greeter')") && $container->hasDefinition('acme.greeter')); check("AcmeHooks::theme() implements hook_theme through #[Hook('theme')]", attributeArguments($themeHook, Hook::class) === ['theme']); check('hook_theme declares the acme_greeting theme hook', isset($themeHooks['acme_greeting'])); check('acme_greeting declares the variables the controller passes', array_keys($themeHooks['acme_greeting']['variables'] ?? []) === ['name', 'greeting']); diff --git a/examples/drupal/web/modules/custom/acme/acme.module b/examples/drupal/web/modules/custom/acme/acme.module index ba6766136..3df53c3df 100644 --- a/examples/drupal/web/modules/custom/acme/acme.module +++ b/examples/drupal/web/modules/custom/acme/acme.module @@ -7,6 +7,7 @@ declare(strict_types=1); +use Drupal\acme\Greeter; use Drupal\Core\Routing\RouteMatchInterface; /** @@ -17,7 +18,16 @@ function acme_help(string $route_name, RouteMatchInterface $route_match): ?strin // Try: complete after `$route_match->`. The methods come from core's // RouteMatchInterface. if ($route_name === 'help.page.acme' && $route_match->getParameter('name') === 'acme') { - return '

Greets the name in the path, for example /acme/greeting/Ada.

'; + // Try: hover `$greeter`, typed by the class acme.services.yml + // declares, or go to the definition of 'acme.greeter'. Complete a + // service ID inside `\Drupal::service('')`. + $greeter = \Drupal::service('acme.greeter'); + // Try: hover `$sameGreeter`. acme.services.yml also registers the + // service under its class name, and a lookup by class name is typed + // as that class. + $sameGreeter = \Drupal::service(Greeter::class); + + return '

' . $greeter->greet('Ada') . ' ' . $sameGreeter->greet('Grace') . '

'; } return null; diff --git a/examples/drupal/web/modules/custom/acme/acme.services.yml b/examples/drupal/web/modules/custom/acme/acme.services.yml index 38cd86433..6e7dcf517 100644 --- a/examples/drupal/web/modules/custom/acme/acme.services.yml +++ b/examples/drupal/web/modules/custom/acme/acme.services.yml @@ -1,8 +1,13 @@ +parameters: + acme.greeting_format: 'Hello @name, welcome to @site.' + services: acme.greeter: # Try: go to the definition of Greeter, or find its references: this # line is one of them. class: Drupal\acme\Greeter - arguments: ['@config.factory'] + # Try: go to the definition of `@config.factory`, which core declares, + # or of `%acme.greeting_format%`. Complete a service ID after `@`. + arguments: ['@config.factory', '%acme.greeting_format%'] # Lets controllers and hook classes autowire the greeter by its class. Drupal\acme\Greeter: '@acme.greeter' diff --git a/examples/drupal/web/modules/custom/acme/src/Greeter.php b/examples/drupal/web/modules/custom/acme/src/Greeter.php index 7d66da71b..afad6bdd7 100644 --- a/examples/drupal/web/modules/custom/acme/src/Greeter.php +++ b/examples/drupal/web/modules/custom/acme/src/Greeter.php @@ -13,6 +13,7 @@ final class Greeter { public function __construct( private readonly ConfigFactoryInterface $configFactory, + private readonly string $format, ) {} public function greet(string $name): string @@ -21,6 +22,9 @@ public function greet(string $name): string // from core's ConfigFactoryInterface. $siteName = $this->configFactory->get('system.site')->get('name'); - return sprintf('Hello %s, welcome to %s.', $name, is_string($siteName) ? $siteName : 'Drupal'); + return strtr($this->format, [ + '@name' => $name, + '@site' => is_string($siteName) ? $siteName : 'Drupal', + ]); } } diff --git a/examples/drupal/web/modules/custom/acme/src/NoteCounter.php b/examples/drupal/web/modules/custom/acme/src/NoteCounter.php new file mode 100644 index 000000000..173d406d0 --- /dev/null +++ b/examples/drupal/web/modules/custom/acme/src/NoteCounter.php @@ -0,0 +1,37 @@ +get('')`. + return new static($container->get('entity_type.manager')); + } + + public function count(): int + { + return (int) $this->entityTypeManager + ->getStorage('acme_note') + ->getQuery() + ->accessCheck(false) + ->count() + ->execute(); + } +} diff --git a/src/analyse/stages.rs b/src/analyse/stages.rs index 7f2e0ceb9..24180f296 100644 --- a/src/analyse/stages.rs +++ b/src/analyse/stages.rs @@ -188,7 +188,7 @@ pub(super) fn index_project( // only by that walk, and a Yii project's routes are declared in its // `config/`, which no autoloader names. if backend.resolved_class_cache.read().laravel().is_enabled() - || backend.is_symfony_project() + || backend.reads_service_container() || backend.is_twig_project() || backend.is_yii_project() { diff --git a/src/backend/construction.rs b/src/backend/construction.rs index 29be78360..93e533450 100644 --- a/src/backend/construction.rs +++ b/src/backend/construction.rs @@ -112,6 +112,7 @@ impl BackendState { twig: crate::twig::TwigState::default(), yii: crate::yii::YiiState::default(), wordpress_hooks: crate::wordpress::hooks::HookFiles::default(), + drupal: crate::drupal::DrupalState::default(), pest_configs: Arc::new(RwLock::new(None)), member_completion_cache: Arc::new(Mutex::new(HashMap::new())), diag: crate::diagnostics::state::DiagnosticState::new(), diff --git a/src/backend/settings.rs b/src/backend/settings.rs index 1690eb4c2..55e68be12 100644 --- a/src/backend/settings.rs +++ b/src/backend/settings.rs @@ -110,7 +110,8 @@ impl Backend { /// Record that a Composer project of the workspace is a Drupal site, /// so every workspace walk reads Drupal's `.module`, `.install`, … - /// files as PHP the way the web-root class scan does. + /// files as PHP the way the web-root class scan does, and Drupal's own + /// declarations, such as its services, are read (see [`crate::drupal`]). /// /// Only ever turned on: a workspace whose subprojects include one /// Drupal site treats those extensions as PHP throughout. diff --git a/src/blade/typed_receiver.rs b/src/blade/typed_receiver.rs index a4f2d0e8e..72d93f0eb 100644 --- a/src/blade/typed_receiver.rs +++ b/src/blade/typed_receiver.rs @@ -126,6 +126,11 @@ impl Backend { content: &str, sites: &[TypedReceiverSite], ) -> Vec { + // The spans are cached per file, and a caller working on a file of + // another Composer project (a `\Drupal::service()` lookup reading a + // Symfony app's configurator) would otherwise resolve this file's + // receivers against that project's classes. + let _project = self.enter_project_scope(uri); let _resolver_guard = crate::type_engine::call_resolution::activate_type_engine_caches(); let _cache_guard = crate::virtual_members::with_active_resolved_class_cache(&self.resolved_class_cache); diff --git a/src/classmap_scanner/discovery.rs b/src/classmap_scanner/discovery.rs index 07c9221cf..550112171 100644 --- a/src/classmap_scanner/discovery.rs +++ b/src/classmap_scanner/discovery.rs @@ -634,11 +634,14 @@ fn read_phpstorm_stubs_map( /// with `[indexing] exclude`, which is honored here like everywhere /// else; duplicate fixture classes are handled by the classmap's /// first-wins merge. +/// +/// The same walk collects the YAML files Drupal reads declarations from +/// (see `drupal::is_declaration_file`), returned beside the symbols. pub fn scan_drupal_directories( web_root: &Path, filters: &std::sync::Arc, progress: Option<&ScanProgress>, -) -> WorkspaceScanResult { +) -> (WorkspaceScanResult, Vec) { use ignore::WalkBuilder; let drupal_dirs = [ @@ -652,6 +655,7 @@ pub fn scan_drupal_directories( ]; let mut php_files: Vec<(PathBuf, crate::ClassCompletionOrigin)> = Vec::new(); + let mut declaration_files: Vec = Vec::new(); for rel in &drupal_dirs { let dir = web_root.join(rel); @@ -678,14 +682,22 @@ pub fn scan_drupal_directories( for entry in walker.flatten() { let path = entry.path(); - if path.is_file() && (is_drupal_php_file(path) || filters.is_php_file(path)) { + if !path.is_file() { + continue; + } + if is_drupal_php_file(path) || filters.is_php_file(path) { php_files.push((path.to_path_buf(), crate::ClassCompletionOrigin::Project)); + } else if crate::drupal::is_declaration_file(path, web_root) { + declaration_files.push(path.to_path_buf()); } } } progress_add_total(progress, php_files.len()); - scan_files_parallel_full(&php_files, progress) + ( + scan_files_parallel_full(&php_files, progress), + declaration_files, + ) } /// The extensions besides `.php` that Drupal treats as PHP source. diff --git a/src/classmap_scanner/discovery_tests.rs b/src/classmap_scanner/discovery_tests.rs index 28bb15386..c76ebc4da 100644 --- a/src/classmap_scanner/discovery_tests.rs +++ b/src/classmap_scanner/discovery_tests.rs @@ -428,7 +428,7 @@ fn scan_drupal_directories_finds_php_and_module_files() { ) .unwrap(); - let result = scan_drupal_directories(web_root, &IndexFilters::empty(), None); + let (result, _) = scan_drupal_directories(web_root, &IndexFilters::empty(), None); assert!( result .classmap @@ -471,7 +471,7 @@ fn scan_drupal_directories_indexes_test_dirs_by_default() { ) .unwrap(); - let result = scan_drupal_directories(web_root, &IndexFilters::empty(), None); + let (result, _) = scan_drupal_directories(web_root, &IndexFilters::empty(), None); assert!( result.classmap.contains_key("Drupal\\Tests\\UnitTestCase"), "core test base classes must be indexed" @@ -485,7 +485,7 @@ fn scan_drupal_directories_indexes_test_dirs_by_default() { // Projects that want the old behaviour opt in via the exclude list. let filters = test_filters(dir.path(), &["tests/", "Tests/"], &[]); - let result = scan_drupal_directories(web_root, &filters, None); + let (result, _) = scan_drupal_directories(web_root, &filters, None); assert!( !result .classmap @@ -502,7 +502,7 @@ fn scan_drupal_directories_indexes_test_dirs_by_default() { fn scan_drupal_directories_skips_nonexistent_dirs() { let dir = tempfile::tempdir().unwrap(); // Empty web root — none of the expected subdirectories exist - let result = scan_drupal_directories(dir.path(), &IndexFilters::empty(), None); + let (result, _) = scan_drupal_directories(dir.path(), &IndexFilters::empty(), None); assert!(result.classmap.is_empty()); assert!(result.function_index.is_empty()); assert!(result.constant_index.is_empty()); @@ -523,7 +523,7 @@ fn scan_drupal_directories_ignores_non_php_files() { ) .unwrap(); - let result = scan_drupal_directories(web_root, &IndexFilters::empty(), None); + let (result, _) = scan_drupal_directories(web_root, &IndexFilters::empty(), None); // Only the .php file should be indexed assert!( result.function_index.contains_key("install_begin"), @@ -701,7 +701,7 @@ fn drupal_scan_honors_exclude_globs() { .unwrap(); let filters = test_filters(dir.path(), &["fixtures"], &[]); - let result = scan_drupal_directories(web_root, &filters, None); + let (result, _) = scan_drupal_directories(web_root, &filters, None); assert!(result.classmap.contains_key("Mymod")); assert!( @@ -709,3 +709,34 @@ fn drupal_scan_honors_exclude_globs() { "exclude globs should apply to the Drupal scanner too" ); } + +#[test] +fn scan_drupal_directories_collects_services_files_outside_tests() { + let dir = tempfile::tempdir().unwrap(); + let web_root = dir.path(); + for (path, content) in [ + ("core/core.services.yml", "services: {}\n"), + ("modules/contrib/token/token.services.yml", "services: {}\n"), + ("modules/contrib/token/token.info.yml", "name: Token\n"), + ("sites/default/services.yml", "parameters: {}\n"), + ( + "core/modules/system/tests/modules/fixture/fixture.services.yml", + "services: {}\n", + ), + ] { + let full = web_root.join(path); + std::fs::create_dir_all(full.parent().unwrap()).unwrap(); + std::fs::write(full, content).unwrap(); + } + + let (_, mut files) = scan_drupal_directories(web_root, &IndexFilters::empty(), None); + files.sort(); + assert_eq!( + files, + vec![ + web_root.join("core/core.services.yml"), + web_root.join("modules/contrib/token/token.services.yml"), + web_root.join("sites/default/services.yml"), + ] + ); +} diff --git a/src/completion/handler/string_arguments.rs b/src/completion/handler/string_arguments.rs index 25cf2a2d5..e05c2082c 100644 --- a/src/completion/handler/string_arguments.rs +++ b/src/completion/handler/string_arguments.rs @@ -101,7 +101,7 @@ impl Backend { // `$twig->render('|')`, and in a Yii one `$urls->generate('|')` and // `#[RouteArgument('|')]`. if matches!(string_ctx, StringContext::InStringLiteral) - && (self.is_symfony_project() || self.is_twig_project() || self.is_yii_project()) + && (self.reads_service_container() || self.is_twig_project() || self.is_yii_project()) && let Some(code) = code_ctx.as_ref() && let Some(response) = self.try_symfony_string_key_completion(uri, content, position, code, ctx) diff --git a/src/completion/symfony_string_keys.rs b/src/completion/symfony_string_keys.rs index f4bb75531..6c740efda 100644 --- a/src/completion/symfony_string_keys.rs +++ b/src/completion/symfony_string_keys.rs @@ -235,7 +235,12 @@ impl Backend { }; }; if call.is_static { - return None; + // `\Drupal::service('|')` and `\Drupal::hasService('|')`. + let class = ctx.resolve_name_at(subject, cursor as u32); + return (call.arg_index == 0 + && self.is_drupal_project() + && crate::drupal::is_service_lookup(&class, &call.method_name)) + .then_some(StringKeyKind::ServiceId); } self.receiver_call_key( subject, @@ -407,6 +412,8 @@ impl Backend { return (!items.is_empty()) .then_some(CompletionResponse::CompletionItemList(items)); } + } else if self.is_drupal_services_uri(uri) { + resource_key_at(content, cursor)? } else if !is_symfony { return None; } else if crate::symfony::resources::is_container_config_uri(uri) { diff --git a/src/composer/project.rs b/src/composer/project.rs index ad987bbb8..bcaaa8d7f 100644 --- a/src/composer/project.rs +++ b/src/composer/project.rs @@ -225,6 +225,14 @@ pub(crate) fn is_yii_project(package: &ComposerPackage) -> bool { }) } +/// Whether the package is a Drupal project: one that requires Drupal core. +pub(crate) fn is_drupal_project(package: &ComposerPackage) -> bool { + const DRUPAL_PACKAGES: &[&str] = &["drupal/core", "drupal/core-recommended", "drupal/core-dev"]; + DRUPAL_PACKAGES + .iter() + .any(|pkg| package.require.contains_key(*pkg) || package.require_dev.contains_key(*pkg)) +} + /// Detect whether the project is a Laravel *application* rather than a /// library that uses an Illuminate component. /// @@ -378,13 +386,7 @@ pub(crate) fn detect_drupal_web_root( workspace_root: &Path, package: &ComposerPackage, ) -> Option { - const DRUPAL_PACKAGES: &[&str] = &["drupal/core", "drupal/core-recommended", "drupal/core-dev"]; - - let is_drupal = DRUPAL_PACKAGES - .iter() - .any(|pkg| package.require.contains_key(*pkg) || package.require_dev.contains_key(*pkg)); - - if !is_drupal { + if !is_drupal_project(package) { return None; } diff --git a/src/diagnostics/collect.rs b/src/diagnostics/collect.rs index 49e6109cb..43262e8dd 100644 --- a/src/diagnostics/collect.rs +++ b/src/diagnostics/collect.rs @@ -473,7 +473,7 @@ impl Backend { let is_laravel = self.resolved_class_cache.read().laravel().is_enabled(); let has_string_keys = is_laravel - || self.is_symfony_project() + || self.reads_service_container() || self.is_twig_project() || self.is_yii_project(); for collector in SLOW_COLLECTORS { diff --git a/src/diagnostics/laravel_string_keys.rs b/src/diagnostics/laravel_string_keys.rs index ab3065b9e..8a4211fb3 100644 --- a/src/diagnostics/laravel_string_keys.rs +++ b/src/diagnostics/laravel_string_keys.rs @@ -826,6 +826,15 @@ impl Backend { if *is_write || *is_optional || !self.string_key_kind_enabled(*kind) { return None; } + // Only Symfony's naming tells an application's own service + // IDs and parameters from the ones its bundles register; + // Drupal reads the same names with no such rule to judge + // them by. + if kind.framework() == crate::symbol_map::KeyFramework::ServiceContainer + && !self.is_symfony_project() + { + return None; + } Some(( CheckedStringKind::of(kind)?, key.as_str(), diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs new file mode 100644 index 000000000..7e4ff16c3 --- /dev/null +++ b/src/drupal/mod.rs @@ -0,0 +1,186 @@ +//! Drupal's own declarations, read the way Drupal reads them. +//! +//! Drupal builds a Symfony service container from YAML in Symfony's +//! service format, but finds the files itself: every module, theme, and +//! install profile under the web root declares its services in +//! `.services.yml`, core in `core/core.services.yml`, and the site in +//! `sites/*/services.yml`. Those are read by the same scan as Symfony's +//! configuration (see [`crate::symfony::resources`]), so service IDs and +//! container parameters work alike in both. Nothing else Symfony declares +//! (routes, translations, events) is read here: Drupal declares those its +//! own way. + +use std::path::{Path, PathBuf}; +use std::sync::atomic::Ordering; + +use parking_lot::RwLock; + +pub(crate) mod types; + +use crate::Backend; + +/// The web roots of the workspace's Drupal projects, and the declaration +/// files found under them. +#[derive(Default)] +pub(crate) struct DrupalState { + web_roots: RwLock>, + /// Core's and contributed extensions' declaration files sit in folders + /// the workspace's `.gitignore` leaves out, so the walk that finds the + /// rest of the project's YAML never reaches them. + declaration_files: RwLock>, +} + +/// Whether the file name at the end of `path` is one Drupal loads services +/// from: `services.yml`, or one ending in `.services.yml` other than +/// `default.services.yml`, the template a site's `services.yml` is copied +/// from. +fn is_services_file_name(path: &str) -> bool { + let name = path.rsplit('/').next().unwrap_or(path); + name == "services.yml" || (name.ends_with(".services.yml") && name != "default.services.yml") +} + +/// The folders of a web root whose YAML Drupal never loads as it stands: +/// core's scaffold sources, which Composer copies into `sites/`, and the +/// sites functional tests generate. +const UNLOADED_DIRS: [&str; 2] = ["core/assets", "sites/simpletest"]; + +/// Whether `path` lies in one of the [`UNLOADED_DIRS`] of `web_root`. +fn is_unloaded(path: &Path, web_root: &Path) -> bool { + UNLOADED_DIRS + .iter() + .any(|dir| path.starts_with(web_root.join(dir))) +} + +/// The folders of a web root that Composer installs and overwrites: core, +/// and the contributed extensions and libraries. +const READ_ONLY_DIRS: [&str; 5] = [ + "core", + "modules/contrib", + "themes/contrib", + "profiles/contrib", + "libraries", +]; + +/// Whether a static call of `method` on `class`, a resolved class name, +/// looks a service up by its ID: `\Drupal::service()` or +/// `\Drupal::hasService()`. Only the global `Drupal` class counts. +pub(crate) fn is_service_lookup(class: &str, method: &str) -> bool { + class.strip_prefix('\\').unwrap_or(class) == "Drupal" + && (method.eq_ignore_ascii_case("service") || method.eq_ignore_ascii_case("hasService")) +} + +/// Whether `path` is a file Drupal reads declarations from. +pub(crate) fn is_declaration_file(path: &Path, web_root: &Path) -> bool { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(is_services_file_name) + && !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. +fn is_test_fixture(path: &Path, web_root: &Path) -> bool { + path.strip_prefix(web_root) + .unwrap_or(path) + .components() + .any(|part| part.as_os_str() == "tests") +} + +impl Backend { + /// Whether the workspace holds a Drupal project: one whose web root + /// was detected; see [`Backend::mark_drupal_project`]. + pub(crate) fn is_drupal_project(&self) -> bool { + self.workspace.is_drupal.load(Ordering::Acquire) + } + + /// Whether the project builds a Symfony service container, as Symfony + /// and Drupal both do: service IDs and container parameters are read in + /// either. + pub(crate) fn reads_service_container(&self) -> bool { + self.is_symfony_project() || self.is_drupal_project() + } + + /// Forget the web roots and their declaration files, before the + /// workspace's Composer projects are read again. + pub(crate) fn clear_drupal_web_roots(&self) { + self.drupal.web_roots.write().clear(); + self.drupal.declaration_files.write().clear(); + } + + /// Remember the declaration files the Drupal walk found. + pub(crate) fn note_drupal_declaration_files(&self, files: Vec) { + self.drupal.declaration_files.write().extend(files); + } + + /// The declaration files the Drupal walk found, for the workspace index + /// to read with the rest of the project's YAML. + pub(crate) fn drupal_declaration_files(&self) -> Vec { + self.drupal.declaration_files.read().clone() + } + + /// Remember the web root of a Drupal project. + pub(crate) fn note_drupal_web_root(&self, web_root: &Path) { + let mut roots = self.drupal.web_roots.write(); + if !roots.iter().any(|root| root == web_root) { + roots.push(web_root.to_path_buf()); + } + } + + /// Whether `uri` is code Composer installs and overwrites: a vendor + /// package, or Drupal core or a contributed extension in the web root. + /// Rename and move never edit it. + pub(crate) fn is_installed_package_uri(&self, uri: &str, vendor_prefixes: &[String]) -> bool { + vendor_prefixes + .iter() + .any(|prefix| uri.starts_with(prefix.as_str())) + || self.is_drupal_read_only_uri(uri) + } + + /// Whether `uri` is a file of Drupal core or of a contributed extension, + /// which Composer overwrites on its next update: read for what it + /// declares, but never edited. + pub(crate) fn is_drupal_read_only_uri(&self, uri: &str) -> bool { + if !self.is_drupal_project() { + return false; + } + let Some(path) = crate::util::uri_to_path(uri) else { + return false; + }; + self.drupal.web_roots.read().iter().any(|root| { + READ_ONLY_DIRS + .iter() + .any(|dir| path.starts_with(root.join(dir))) + }) + } + + /// Whether `uri` is a file a Drupal project loads services from: a + /// services file inside one of its web roots, outside test modules + /// unless it is open. + /// + /// Other YAML files in a Drupal project have a `services:` key with + /// another meaning, or none at all. + pub(crate) fn is_drupal_services_uri(&self, uri: &str) -> bool { + if !self.is_drupal_project() || !is_services_file_name(uri) { + return false; + } + let Some(path) = crate::util::uri_to_path(uri) else { + return false; + }; + let Some((in_fixture, unloaded)) = self + .drupal + .web_roots + .read() + .iter() + .find(|root| path.starts_with(root)) + .map(|root| (is_test_fixture(&path, root), is_unloaded(&path, root))) + else { + return false; + }; + !unloaded && (!in_fixture || self.open_files.read().contains_key(uri)) + } +} + +#[cfg(test)] +mod tests; diff --git a/src/drupal/tests.rs b/src/drupal/tests.rs new file mode 100644 index 000000000..937a12e91 --- /dev/null +++ b/src/drupal/tests.rs @@ -0,0 +1,247 @@ +use std::path::Path; + +use crate::symbol_map::StringKeyKind; + +/// A Drupal project's backend whose web root is `web/` under the +/// workspace, with `files` published the way the workspace index publishes +/// them. +fn drupal_backend(files: &[(&str, &str)]) -> (crate::Backend, tempfile::TempDir) { + backend_with_web_root(files, true) +} + +/// A backend with a web root `web/` under the workspace, marked as a +/// Drupal project only when `is_drupal` holds. +fn backend_with_web_root( + files: &[(&str, &str)], + is_drupal: bool, +) -> (crate::Backend, tempfile::TempDir) { + let dir = tempfile::tempdir().unwrap(); + let backend = crate::Backend::new_test_with_workspace(dir.path().to_path_buf(), vec![]); + backend.set_is_symfony(false); + if is_drupal { + backend.mark_drupal_project(); + } + backend.note_drupal_web_root(&dir.path().join("web")); + for (path, content) in files { + let uri = uri(dir.path(), path); + backend.update_resource_symbol_index(&uri, content); + } + (backend, dir) +} + +fn uri(root: &Path, path: &str) -> String { + crate::util::path_to_uri(&root.join(path)) +} + +const SERVICES: &str = "parameters:\n acme.limit: 10\nservices:\n acme.greeter:\n class: Drupal\\acme\\Greeter\n arguments: ['@config.factory']\n"; + +#[test] +fn services_files_in_the_web_root_declare_services_and_parameters() { + let (backend, _dir) = drupal_backend(&[ + ("web/modules/custom/acme/acme.services.yml", SERVICES), + ( + "web/core/core.services.yml", + "services:\n config.factory:\n class: Drupal\\Core\\Config\\ConfigFactory\n", + ), + ( + "web/sites/default/services.yml", + "parameters:\n session.storage.options: {}\n", + ), + ]); + assert_eq!( + backend.symfony_declared_names(StringKeyKind::ServiceId), + vec!["acme.greeter".to_string(), "config.factory".to_string()] + ); + assert_eq!( + backend.symfony_declared_names(StringKeyKind::ContainerParameter), + vec![ + "acme.limit".to_string(), + "session.storage.options".to_string() + ] + ); + let classes: Vec = backend + .symfony_service_classes("acme.greeter") + .iter() + .map(ToString::to_string) + .collect(); + assert_eq!(classes, vec!["Drupal\\acme\\Greeter".to_string()]); +} + +#[test] +fn other_yaml_is_not_read_as_services() { + let (backend, _dir) = drupal_backend(&[ + // A file with a `services:` key that Drupal does not load services from. + ( + "web/modules/custom/acme/acme.info.yml", + "services:\n not.a_service: ~\n", + ), + ("docker-compose.yml", "services:\n php: ~\n"), + // A services file outside every web root. + ("tools/acme.services.yml", "services:\n tools.thing: ~\n"), + // Templates and copies Drupal never loads as they stand. + ( + "web/sites/default/default.services.yml", + "services:\n template.thing: ~\n", + ), + ( + "web/core/assets/scaffold/files/development.services.yml", + "services:\n scaffold.thing: ~\n", + ), + ( + "web/sites/simpletest/123/services.yml", + "services:\n test.thing: ~\n", + ), + ]); + assert!( + backend + .symfony_declared_names(StringKeyKind::ServiceId) + .is_empty() + ); +} + +#[test] +fn drupal_reads_the_service_container_and_nothing_else_from_symfony() { + let (backend, _dir) = drupal_backend(&[]); + assert!(backend.string_key_kind_enabled(StringKeyKind::ServiceId)); + assert!(backend.string_key_kind_enabled(StringKeyKind::ContainerParameter)); + for kind in [ + StringKeyKind::SymfonyRoute, + StringKeyKind::SymfonyTranslation, + StringKeyKind::SymfonyEvent, + StringKeyKind::MessengerBus, + StringKeyKind::SymfonyConfigKey, + ] { + assert!(!backend.string_key_kind_enabled(kind), "{kind:?}"); + } +} + +#[test] +fn a_symfony_config_directory_means_nothing_in_drupal() { + let (backend, _dir) = + drupal_backend(&[("config/services.yaml", "services:\n app.mailer: ~\n")]); + assert!( + backend + .symfony_declared_names(StringKeyKind::ServiceId) + .is_empty() + ); +} + +#[test] +fn nothing_is_read_outside_a_drupal_project() { + let (backend, dir) = backend_with_web_root(&[], false); + backend.update_resource_symbol_index( + &uri(dir.path(), "web/modules/custom/acme/acme.services.yml"), + SERVICES, + ); + assert!( + backend + .symfony_declared_names(StringKeyKind::ServiceId) + .is_empty() + ); +} + +#[test] +fn a_test_modules_services_are_read_only_while_it_is_open() { + let path = "web/core/modules/system/tests/modules/fixture/fixture.services.yml"; + let content = "services:\n fixture.thing: ~\n"; + let (backend, dir) = drupal_backend(&[(path, content)]); + assert!( + backend + .symfony_declared_names(StringKeyKind::ServiceId) + .is_empty() + ); + + let uri = uri(dir.path(), path); + backend + .open_files + .write() + .insert(uri.clone(), std::sync::Arc::new(content.to_string())); + backend.update_resource_symbol_index(&uri, content); + assert_eq!( + backend.symfony_declared_names(StringKeyKind::ServiceId), + vec!["fixture.thing".to_string()] + ); +} + +#[test] +fn core_and_contrib_are_read_only_and_custom_code_is_not() { + let (backend, dir) = drupal_backend(&[]); + for (path, read_only) in [ + ("web/core/lib/Drupal.php", true), + ("web/modules/contrib/token/token.services.yml", true), + ("web/themes/contrib/gin/gin.theme", true), + ("web/profiles/contrib/demo/demo.profile", true), + ("web/libraries/chart/chart.php", true), + ("web/modules/custom/acme/acme.services.yml", false), + ("web/themes/custom/acme_theme/acme_theme.theme", false), + ("web/sites/default/services.yml", false), + ("tools/core/script.php", false), + ] { + assert_eq!( + backend.is_drupal_read_only_uri(&uri(dir.path(), path)), + read_only, + "{path}" + ); + } +} + +#[test] +fn moves_and_namespace_renames_leave_contrib_alone() { + let (backend, dir) = drupal_backend(&[]); + let contrib = uri(dir.path(), "web/modules/contrib/token/src/Token.php"); + backend.update_ast( + &contrib, + "method()`). + +use std::sync::Arc; + +use crate::Backend; +use crate::class_lookup::{Fqn, ResolvedClassLoader}; +use crate::types::ClassInfo; + +/// The classes `\Drupal::service(id)` returns, when `class` is the global +/// `Drupal` class and the project declares `id`. +pub(crate) fn service_classes( + backend: &Backend, + class: &str, + method_name: &str, + id: &str, + class_loader: &ResolvedClassLoader<'_>, +) -> Option>> { + if !method_name.eq_ignore_ascii_case("service") + || class.strip_prefix('\\').unwrap_or(class) != "Drupal" + || !backend.is_drupal_project() + { + return None; + } + let classes: Vec> = backend + .symfony_service_classes(id) + .iter() + .filter_map(|class| class_loader(Fqn::new(class))) + .collect(); + (!classes.is_empty()).then_some(classes) +} diff --git a/src/indexing/init.rs b/src/indexing/init.rs index 5285ef31e..7673b9ee3 100644 --- a/src/indexing/init.rs +++ b/src/indexing/init.rs @@ -34,18 +34,42 @@ impl Backend { composer_package: Option, progress: Option<&crate::progress::ScanProgress>, ) { + let was_drupal = self.is_drupal_project(); if composer_package.is_some() { self.init_single_project(root, php_version, composer_package, progress) .await; - return; + } else { + let subprojects = composer::discover_subproject_roots(root); + if subprojects.is_empty() { + self.init_no_composer(root, php_version, progress).await; + } else { + self.init_monorepo(root, &subprojects, php_version, progress) + .await; + } + } + // The editor opens files while the walk runs, and a file read + // before the walk found a Drupal site was read without Drupal's + // names in it. + if !was_drupal && self.is_drupal_project() { + self.reparse_open_files(); } + } - let subprojects = composer::discover_subproject_roots(root); - if subprojects.is_empty() { - self.init_no_composer(root, php_version, progress).await; - } else { - self.init_monorepo(root, &subprojects, php_version, progress) - .await; + /// Read every open file again from its buffer, as opening it does. + fn reparse_open_files(&self) { + let open: Vec<(String, std::sync::Arc)> = self + .open_files + .read() + .iter() + .map(|(uri, text)| (uri.clone(), std::sync::Arc::clone(text))) + .collect(); + for (uri, text) in open { + if self.is_resource_uri(&uri) { + self.update_resource_symbol_index(&uri, &text); + } else { + self.update_ast(&uri, &text); + } + self.schedule_diagnostics(uri); } } @@ -134,6 +158,7 @@ impl Backend { .filter_map(composer::locked_phpunit_major) .min(), ); + self.clear_drupal_web_roots(); // A permission package answers authorization checks from the database, // so the abilities this project uses are not written in its source and @@ -350,11 +375,13 @@ impl Backend { // depend on. detect_drupal_web_root() returns None for // non-Drupal projects so this block is a no-op in that case. if let Some(drupal_web_root) = drupal_web_root { + self.note_drupal_web_root(&drupal_web_root); if let Some(p) = progress { p.set_scope(at(55), at(59), "Scanning Drupal directories"); } - let drupal_result = + let (drupal_result, declaration_files) = classmap_scanner::scan_drupal_directories(&drupal_web_root, &filters, progress); + self.note_drupal_declaration_files(declaration_files); let drupal_count = drupal_result.classmap.len() + drupal_result.function_index.len() + drupal_result.constant_index.len(); @@ -460,6 +487,7 @@ impl Backend { let mut any_twig = false; let mut any_yii = false; let mut phpunit_major: Option = None; + self.clear_drupal_web_roots(); for (sub_idx, (sub_root, _)) in subprojects.iter().enumerate() { // Each subproject owns an equal slice of the 10..80 range; diff --git a/src/indexing/preload.rs b/src/indexing/preload.rs index f24be189c..17620f1d2 100644 --- a/src/indexing/preload.rs +++ b/src/indexing/preload.rs @@ -286,7 +286,7 @@ impl Backend { self.report_workspace_index_progress(progress, 3, "Scanning workspace files"); let walk_start = std::time::Instant::now(); let filters = self.index_filters(); - let (mut php_files, resource_files) = + let (mut php_files, mut resource_files) = crate::classmap_scanner::collect_workspace_index_files_gitignore( &root, &vendor_dir_paths, @@ -310,6 +310,17 @@ impl Backend { .filter(|path| seen.insert(path.clone())), ); } + // Drupal's own walk found the declaration files the gitignore-aware + // walk skips; the custom ones both walks reach are listed once. + let drupal_files = self.drupal_declaration_files(); + if !drupal_files.is_empty() { + let mut seen: HashSet = resource_files.iter().cloned().collect(); + resource_files.extend( + drupal_files + .into_iter() + .filter(|path| seen.insert(path.clone())), + ); + } tracing::info!( "ensure_workspace_indexed: Phase 2 disk walk found {} PHP and {} resource files in {:?}", php_files.len(), diff --git a/src/lib.rs b/src/lib.rs index 4e5a83941..bf0ea3c19 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,6 +263,7 @@ pub mod diagnostics; pub mod docblock; mod document_links; mod document_symbols; +mod drupal; mod file_state; pub mod fix; mod folding; @@ -538,6 +539,8 @@ pub struct BackendState { pub(crate) yii: yii::YiiState, /// The files that fire or use a WordPress hook. pub(crate) wordpress_hooks: wordpress::hooks::HookFiles, + /// The web roots of the workspace's Drupal projects. + pub(crate) drupal: drupal::DrupalState, /// The `Pest.php` files of the project's test directories, read on /// first use and dropped when one is saved. `None` until then. pub(crate) pest_configs: Arc>>, diff --git a/src/parser/ast_update.rs b/src/parser/ast_update.rs index b33207af3..e1cdda391 100644 --- a/src/parser/ast_update.rs +++ b/src/parser/ast_update.rs @@ -561,8 +561,12 @@ impl Backend { program, content, &owned_resolved, - self.is_symfony_project(), - self.is_twig_project(), + crate::symbol_map::extraction::Frameworks { + symfony: self.is_symfony_project(), + service_container: self.reads_service_container(), + drupal: self.is_drupal_project(), + twig: self.is_twig_project(), + }, yii_dir.as_deref(), ) }; diff --git a/src/reference_index.rs b/src/reference_index.rs index 82802e8c1..941a9d180 100644 --- a/src/reference_index.rs +++ b/src/reference_index.rs @@ -18,7 +18,7 @@ use crate::Backend; use crate::atom::{Atom, AtomMap, AtomSet, atom}; use crate::backend::file_access::namespace_in_spans; use crate::names::resolve_to_fqn; -use crate::symbol_map::{SelfStaticParentKind, StringKeyKind, SymbolKind, SymbolMap}; +use crate::symbol_map::{KeyFramework, SelfStaticParentKind, StringKeyKind, SymbolKind, SymbolMap}; use crate::types::NamespaceSpan; use crate::util::{build_fqn, short_name, strip_fqn_prefix}; @@ -740,18 +740,16 @@ impl Backend { // `$x->prop === 'literal'`, so skip them there. let is_laravel = self.resolved_class_cache.read().laravel().is_enabled(); let is_symfony = self.is_symfony_project(); + let reads_container = self.reads_service_container(); let is_twig = self.is_twig_project(); let is_yii = self.is_yii_project(); - let enabled = |kind: StringKeyKind| { - if kind.is_twig() { - is_twig - } else if kind.is_yii() { - is_yii - } else if kind.is_symfony() { - is_symfony - } else { - is_laravel - } + let enabled = |kind: StringKeyKind| match kind.framework() { + KeyFramework::Laravel => is_laravel, + KeyFramework::ServiceContainer => reads_container, + KeyFramework::Symfony => is_symfony, + KeyFramework::Twig => is_twig, + KeyFramework::Yii => is_yii, + KeyFramework::WordPress => false, }; for site in &symbol_map.typed_receiver_sites { if !site.receiver.names_any(enabled) { diff --git a/src/references/dispatch.rs b/src/references/dispatch.rs index 524822ad6..c2da58bc3 100644 --- a/src/references/dispatch.rs +++ b/src/references/dispatch.rs @@ -55,13 +55,17 @@ impl Backend { include_declaration: bool, ) -> Option> { self.ensure_workspace_index_ready_for_request(); - self.find_references_inner( + let mut locations = self.find_references_inner( uri, content, position, include_declaration, ReferenceSearchMode::Rename, - ) + )?; + // Drupal core and contributed extensions are references worth + // finding but files a rename must not edit: Composer overwrites them. + locations.retain(|location| !self.is_drupal_read_only_uri(location.uri.as_str())); + Some(locations) } fn find_references_inner( diff --git a/src/rename/class/move_edits.rs b/src/rename/class/move_edits.rs index 8b2da1adc..cbbd8dd11 100644 --- a/src/rename/class/move_edits.rs +++ b/src/rename/class/move_edits.rs @@ -37,11 +37,7 @@ impl Backend { // package, and moving one headlessly is no safer: the edits would // land in a tree the next `composer install` overwrites. if self - .workspace - .vendor_uri_prefixes - .lock() - .iter() - .any(|prefix| definition_uri.starts_with(prefix.as_str())) + .is_installed_package_uri(&definition_uri, &self.workspace.vendor_uri_prefixes.lock()) { return Err(format!( "`{old_fqn}` is declared in an installed package and cannot be moved." diff --git a/src/rename/namespace/mod.rs b/src/rename/namespace/mod.rs index 491b1d5b8..88a35e099 100644 --- a/src/rename/namespace/mod.rs +++ b/src/rename/namespace/mod.rs @@ -87,6 +87,26 @@ impl Backend { return Err(conflict); } + // In a Drupal site, a namespace core or a contributed extension + // declares cannot be renamed in part: the declarations there would + // be left behind, and Composer overwrites them anyway. Vendor + // packages are skipped below, in every project. + if self.is_drupal_project() { + let member_prefix = format!("{old_prefix}\\"); + let installed = self.symbols.fqn_uri_index.read().iter().any(|(fqn, uri)| { + fqn.len() > member_prefix.len() + && fqn + .get(..member_prefix.len()) + .is_some_and(|start| start.eq_ignore_ascii_case(&member_prefix)) + && self.is_drupal_read_only_uri(uri) + }); + if installed { + return Err(format!( + "The namespace `{old_prefix}` is declared in installed code, Drupal core or a contributed extension, and cannot be renamed." + )); + } + } + let mut changes: HashMap> = HashMap::new(); // Scan all known files. The live per-file maps only contain files @@ -124,10 +144,7 @@ impl Backend { let vendor_prefixes = self.workspace.vendor_uri_prefixes.lock().clone(); for file_uri in &all_uris { - if vendor_prefixes - .iter() - .any(|p| file_uri.starts_with(p.as_str())) - { + if self.is_installed_package_uri(file_uri, &vendor_prefixes) { continue; } // A YAML/XML occurrence may be spelled the escaped `App\\Foo` diff --git a/src/rename/prepare.rs b/src/rename/prepare.rs index cf0f4ddf4..07c5d23e0 100644 --- a/src/rename/prepare.rs +++ b/src/rename/prepare.rs @@ -735,7 +735,8 @@ impl Backend { } /// Check whether the symbol under the cursor is defined in a vendor - /// file. + /// file, or in Drupal core or a contributed extension, which Composer + /// installs into the web root instead. /// /// We check this by resolving the definition location. If the /// definition URI starts with the vendor prefix, the rename is @@ -743,21 +744,13 @@ impl Backend { fn is_vendor_symbol(&self, uri: &str, content: &str, position: Position) -> bool { let vendor_prefixes = self.workspace.vendor_uri_prefixes.lock().clone(); - if vendor_prefixes.is_empty() { + if vendor_prefixes.is_empty() && !self.is_drupal_project() { return false; } - for loc in self.resolve_definition(uri, content, position) { - let def_uri = loc.uri.to_string(); - if vendor_prefixes - .iter() - .any(|p| def_uri.starts_with(p.as_str())) - { - return true; - } - } - - false + self.resolve_definition(uri, content, position) + .iter() + .any(|loc| self.is_installed_package_uri(loc.uri.as_str(), &vendor_prefixes)) } /// Determine whether this rename targets a property (as opposed to diff --git a/src/resource_navigation.rs b/src/resource_navigation.rs index bd9e5c430..859d6fbcc 100644 --- a/src/resource_navigation.rs +++ b/src/resource_navigation.rs @@ -275,7 +275,15 @@ impl Backend { let mut container_values = Vec::new(); let mut route_imports = Vec::new(); let mut declares_twig_loader = false; - if is_symfony && crate::symfony::resources::is_container_config_uri(uri) { + // A Drupal declaration file is Drupal's, also in a Drupal site that + // requires Symfony's container and has a `config` directory on the + // way to it, as core's `config` module does. + if self.is_drupal_services_uri(uri) { + let (container_spans, values) = + crate::symfony::resources::container_declarations(uri, content); + spans = container_spans; + container_values = values; + } else if is_symfony && crate::symfony::resources::is_container_config_uri(uri) { declares_twig_loader = crate::symfony::tags::tags_twig_loader(content); // A bus is declared by a key under `config/packages/`, which is // a configuration key too; the bus is the one that means diff --git a/src/symbol_map/extraction/expressions/calls.rs b/src/symbol_map/extraction/expressions/calls.rs index 1604f0589..595046ef7 100644 --- a/src/symbol_map/extraction/expressions/calls.rs +++ b/src/symbol_map/extraction/expressions/calls.rs @@ -599,6 +599,14 @@ fn extract_static_method_call<'a>( &static_call.argument_list, ctx, ); + if ctx.drupal && crate::drupal::is_service_lookup(semantic_subject, &member_name) { + try_emit_laravel_string_span( + crate::symbol_map::StringKeyKind::ServiceId, + &static_call.argument_list, + ctx.content, + &mut ctx.spans, + ); + } } extract_from_arguments(&static_call.argument_list.arguments, ctx, scope_start); } @@ -678,6 +686,50 @@ mod tests { use crate::names::OwnedResolvedNames; use crate::symbol_map::StringKeyDependency; + /// The service IDs `\Drupal::service()` names in `php`, extracted for a + /// project that builds a Symfony service container, and is a Drupal + /// site when `drupal` holds. + fn drupal_service_ids(php: &str, drupal: bool) -> Vec { + let arena = mago_allocator::LocalArena::new(); + let file_id = mago_database::file::FileId::new(b"test.php"); + let program = mago_syntax::parser::parse_file_content(&arena, file_id, php.as_bytes()); + let resolver = mago_names::resolver::NameResolver::new(&arena); + let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); + let map = crate::symbol_map::extraction::extract_symbol_map_for_index( + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: true, + service_container: true, + drupal, + twig: false, + }, + None, + ); + map.spans + .iter() + .filter_map(|span| match &span.kind { + crate::symbol_map::SymbolKind::StringKey { + kind: crate::symbol_map::StringKeyKind::ServiceId, + key, + .. + } => Some(key.clone()), + _ => None, + }) + .collect() + } + + #[test] + fn drupal_service_lookups_name_a_service_only_in_a_drupal_site() { + let php = "can('update', 'post');\n"; @@ -688,7 +740,16 @@ mod tests { let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); let map = crate::symbol_map::extraction::extract_symbol_map_for_index( - program, php, &names, true, true, None, + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: true, + service_container: true, + drupal: false, + twig: true, + }, + None, ); assert_eq!(map.conditional_string_key_spans.len(), 1); diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index da3f8e742..79ce960c2 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -141,6 +141,13 @@ struct ExtractionCtx<'a> { /// `get()` and `set()` would otherwise record all over any other /// project. symfony: bool, + /// Whether the project builds a Symfony service container, as Symfony + /// and Drupal both do. Gates the container lookups recorded for the + /// type engine to confirm as naming a service or a parameter. + service_container: bool, + /// Whether the project is a Drupal site. Gates the lookups only + /// Drupal makes, such as `\Drupal::service()`. + drupal: bool, /// Whether the project uses Twig, outside Symfony too. Gates the /// `render()` calls recorded for the type engine to confirm as naming /// a template. @@ -355,13 +362,39 @@ fn descend_unhandled<'a>(node: Node<'a, 'a>, ctx: &mut ExtractionCtx<'a>, scope_ // ─── AST extraction ───────────────────────────────────────────────────────── +/// The frameworks whose names a symbol map records, by what the project +/// uses. +#[derive(Debug, Clone, Copy)] +pub(crate) struct Frameworks { + /// The project is a Symfony application. + pub symfony: bool, + /// The project builds a Symfony service container: a Symfony or a + /// Drupal project. + pub service_container: bool, + /// The project is a Drupal project. + pub drupal: bool, + /// The project uses Twig. + pub twig: bool, +} + +#[cfg(test)] +impl Frameworks { + /// Every framework, for tests that do not care which project they are in. + pub(crate) const ALL: Self = Self { + symfony: true, + service_container: true, + drupal: true, + twig: true, + }; +} + /// Build a [`SymbolMap`] from a parsed PHP program. /// /// Walks every statement recursively and emits [`SymbolSpan`] entries for /// every navigable symbol occurrence. #[cfg(test)] pub(crate) fn extract_symbol_map(program: &Program<'_>, content: &str) -> SymbolMap { - extract_symbol_map_inner(program, content, None, true, true, None) + extract_symbol_map_inner(program, content, None, Frameworks::ALL, None) } /// Build a [`SymbolMap`] using the semantic names produced by `mago-names`. @@ -371,8 +404,13 @@ pub(crate) fn extract_symbol_map_with_resolved_names( content: &str, resolved_names: &OwnedResolvedNames, ) -> SymbolMap { - let mut map = - extract_symbol_map_inner(program, content, Some(resolved_names), true, true, None); + let mut map = extract_symbol_map_inner( + program, + content, + Some(resolved_names), + Frameworks::ALL, + None, + ); map.refresh_conditional_string_key_spans(None, |_| false); map } @@ -380,35 +418,31 @@ pub(crate) fn extract_symbol_map_with_resolved_names( /// Build a semantic map while retaining workspace-dependent Laravel spans as /// dormant candidates for the publication lifecycle to settle. /// -/// `symfony` says whether the project builds a Symfony service container, -/// `twig` whether it uses Twig, and `yii_dir` is the file's directory when -/// the project is a Yii application. +/// `frameworks` says which frameworks' names to record, and `yii_dir` is the +/// file's directory when the project is a Yii application. pub(crate) fn extract_symbol_map_for_index( program: &Program<'_>, content: &str, resolved_names: &OwnedResolvedNames, - symfony: bool, - twig: bool, + frameworks: Frameworks, yii_dir: Option<&std::path::Path>, ) -> SymbolMap { - extract_symbol_map_inner( - program, - content, - Some(resolved_names), - symfony, - twig, - yii_dir, - ) + extract_symbol_map_inner(program, content, Some(resolved_names), frameworks, yii_dir) } fn extract_symbol_map_inner( program: &Program<'_>, content: &str, resolved_names: Option<&OwnedResolvedNames>, - symfony: bool, - twig: bool, + frameworks: Frameworks, yii_dir: Option<&std::path::Path>, ) -> SymbolMap { + let Frameworks { + symfony, + service_container, + drupal, + twig, + } = frameworks; let mut ctx = ExtractionCtx { spans: Vec::new(), var_defs: Vec::new(), @@ -447,6 +481,8 @@ fn extract_symbol_map_inner( in_controller: false, covers_default_class: None, symfony, + service_container, + drupal, twig, uses_symfony_configurators: None, uses_symfony_routing_configurators: None, diff --git a/src/symbol_map/extraction/symfony.rs b/src/symbol_map/extraction/symfony.rs index 0a8b1c928..796910227 100644 --- a/src/symbol_map/extraction/symfony.rs +++ b/src/symbol_map/extraction/symfony.rs @@ -21,8 +21,8 @@ use super::*; use crate::symbol_map::{ - ContainerValue, ContainerValueSite, MessengerHandlerSite, ReceiverClass, RouteImport, - SYMFONY_CONTAINER_CONFIGURATOR_NS, SYMFONY_ROUTING_CONFIGURATOR, StringKeyKind, + ContainerValue, ContainerValueSite, KeyFramework, MessengerHandlerSite, ReceiverClass, + RouteImport, SYMFONY_CONTAINER_CONFIGURATOR_NS, SYMFONY_ROUTING_CONFIGURATOR, StringKeyKind, }; use crate::symfony::translations::{DEFAULT_DOMAIN, is_domain_name, translation_key}; use crate::virtual_members::laravel::helpers::{resolved_string_literal, string_literal_at_range}; @@ -149,7 +149,7 @@ pub(in crate::symbol_map::extraction) fn record_symfony_receiver_sites( argument_list: &ArgumentList<'_>, ctx: &mut ExtractionCtx<'_>, ) { - if !ctx.symfony && !ctx.twig && !ctx.yii { + if !ctx.service_container && !ctx.twig && !ctx.yii { return; } if ctx.symfony && member_name.eq_ignore_ascii_case("trans") { @@ -187,15 +187,17 @@ pub(in crate::symbol_map::extraction) fn record_symfony_receiver_sites( continue; }; // Outside Symfony, only the calls of the framework the project uses - // are left: Twig's own `render()`, or Yii's URL generator. - let records = |kind: StringKeyKind| { - if kind.is_twig() { - ctx.twig - } else if kind.is_yii() { - ctx.yii - } else { - ctx.symfony - } + // are left: Twig's own `render()`, Yii's URL generator, or a Drupal + // container lookup. Only Symfony declares services and parameters + // in PHP; in Drupal a `setParameter()` is far more often a + // validator's or a query's than the container's. + let records = |kind: StringKeyKind| match kind.framework() { + KeyFramework::ServiceContainer if is_write => ctx.symfony, + KeyFramework::ServiceContainer => ctx.service_container, + KeyFramework::Symfony => ctx.symfony, + KeyFramework::Twig => ctx.twig, + KeyFramework::Yii => ctx.yii, + KeyFramework::Laravel | KeyFramework::WordPress => false, }; if !receiver.names_any(records) { continue; @@ -2526,7 +2528,16 @@ mod tests { let resolver = mago_names::resolver::NameResolver::new(&arena); let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); crate::symbol_map::extraction::extract_symbol_map_for_index( - program, php, &names, true, true, None, + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: true, + service_container: true, + drupal: false, + twig: true, + }, + None, ) } @@ -3150,11 +3161,49 @@ mod tests { let resolver = mago_names::resolver::NameResolver::new(&arena); let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); let map = crate::symbol_map::extraction::extract_symbol_map_for_index( - program, php, &names, false, false, None, + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: false, + }, + None, ); assert!(map.typed_receiver_sites.is_empty()); } + #[test] + fn a_drupal_project_records_container_reads_but_no_declarations() { + let php = "get('acme.greeter');\n $container->getParameter('acme.limit');\n $violation->setParameter('@value', $value);\n}\n"; + let arena = mago_allocator::LocalArena::new(); + let file_id = mago_database::file::FileId::new(b"test.php"); + let program = mago_syntax::parser::parse_file_content(&arena, file_id, php.as_bytes()); + let resolver = mago_names::resolver::NameResolver::new(&arena); + let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); + let map = crate::symbol_map::extraction::extract_symbol_map_for_index( + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: true, + drupal: false, + twig: true, + }, + None, + ); + assert_eq!( + receivers(&map), + vec![ + ("acme.greeter", ReceiverClass::ContainerLookup, false), + ("acme.limit", ReceiverClass::ParameterHolder, false), + ] + ); + } + #[test] fn only_renders_are_recorded_in_a_twig_project() { let php = "get('app.mailer');\n $twig->render('index.html');\n}\n"; @@ -3164,7 +3213,16 @@ mod tests { let resolver = mago_names::resolver::NameResolver::new(&arena); let names = OwnedResolvedNames::from_resolved(&resolver.resolve(program)); let map = crate::symbol_map::extraction::extract_symbol_map_for_index( - program, php, &names, false, true, None, + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: true, + }, + None, ); assert_eq!( receivers(&map), diff --git a/src/symbol_map/extraction/twig.rs b/src/symbol_map/extraction/twig.rs index 2d59fb24c..28c47d89e 100644 --- a/src/symbol_map/extraction/twig.rs +++ b/src/symbol_map/extraction/twig.rs @@ -281,7 +281,7 @@ fn class_named(class: &Expression<'_>, ctx: &ExtractionCtx<'_>) -> Option SymbolMap { let arena = mago_allocator::LocalArena::new(); @@ -289,7 +289,18 @@ 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(program, php, &names, false, true, None) + crate::symbol_map::extract_symbol_map_for_index( + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: true, + }, + None, + ) } fn report(map: &SymbolMap, php: &str) -> Vec { @@ -361,7 +372,8 @@ final class AppExtension extends AbstractExtension { .filter(|span| { matches!( &span.kind, - SymbolKind::StringKey { kind, is_write: true, .. } if kind.is_twig() + SymbolKind::StringKey { kind, is_write: true, .. } + if kind.framework() == KeyFramework::Twig ) }) .count(); diff --git a/src/symbol_map/extraction/twig_context.rs b/src/symbol_map/extraction/twig_context.rs index a9f49bc9a..80a83eb9d 100644 --- a/src/symbol_map/extraction/twig_context.rs +++ b/src/symbol_map/extraction/twig_context.rs @@ -349,7 +349,18 @@ 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(program, php, &names, symfony, true, None) + crate::symbol_map::extract_symbol_map_for_index( + program, + php, + &names, + crate::symbol_map::extraction::Frameworks { + symfony, + service_container: symfony, + drupal: false, + twig: true, + }, + None, + ) } /// Each context as the template it renders, then the names it passes. diff --git a/src/symbol_map/extraction/yii_routes.rs b/src/symbol_map/extraction/yii_routes.rs index 916d29329..34e794e29 100644 --- a/src/symbol_map/extraction/yii_routes.rs +++ b/src/symbol_map/extraction/yii_routes.rs @@ -395,8 +395,12 @@ mod tests { program, php, &names, - false, - false, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: false, + }, Some(std::path::Path::new("/p")), ) } diff --git a/src/symbol_map/string_keys.rs b/src/symbol_map/string_keys.rs index b7a3d4c9b..b204324e2 100644 --- a/src/symbol_map/string_keys.rs +++ b/src/symbol_map/string_keys.rs @@ -9,7 +9,7 @@ use super::{LaravelConfigResource, SymbolKind, SymbolMap, SymbolSpan}; /// /// One flat list for every framework: each variant names what its key /// resolves against, so a framework's kinds sit beside the others rather -/// than behind a per-framework wrapper. [`Self::is_symfony`] says which +/// than behind a per-framework wrapper. [`Self::framework`] says which /// framework a variant belongs to. /// /// Adding a new string-key navigation feature only requires adding a variant @@ -168,45 +168,40 @@ impl StringKeyKind { matches!(self, Self::SymfonyTranslation | Self::TranslationDomain) } - /// Whether this key belongs to Symfony rather than to Laravel. - pub(crate) const fn is_symfony(self) -> bool { - matches!( - self, - Self::ServiceId - | Self::ContainerParameter - | Self::SymfonyRoute - | Self::SymfonyTranslation - | Self::TranslationDomain - | Self::TwigTemplate - | Self::SymfonyConfigKey - | Self::SymfonyEvent - | Self::MessengerBus - ) - } - - /// Whether this key belongs to Twig, whatever the framework around it. - pub(crate) const fn is_twig(self) -> bool { - matches!( - self, + /// The framework whose declarations this key resolves against. + /// + /// Exhaustive on purpose: a new kind has to say which framework turns + /// it on. + pub(crate) const fn framework(self) -> KeyFramework { + match self { + Self::Config + | Self::ConfigResource(_) + | Self::View + | Self::Route + | Self::Trans + | Self::Command + | Self::MorphAlias + | Self::Section + | Self::Stack + | Self::GateAbility + | Self::ContainerBinding + | Self::Middleware + | Self::Env => KeyFramework::Laravel, + Self::ServiceId | Self::ContainerParameter => KeyFramework::ServiceContainer, + Self::SymfonyRoute + | Self::SymfonyTranslation + | Self::TranslationDomain + | Self::SymfonyConfigKey + | Self::SymfonyEvent + | Self::MessengerBus => KeyFramework::Symfony, Self::TwigTemplate - | Self::TwigBlock - | Self::TwigFilter - | Self::TwigFunction - | Self::TwigVariable - ) - } - - /// Whether this key belongs to Yii. - pub(crate) const fn is_yii(self) -> bool { - matches!( - self, - Self::YiiView | Self::YiiRoute | Self::YiiRouteArgument - ) - } - - /// Whether this key belongs to WordPress. - pub(crate) const fn is_wordpress(self) -> bool { - matches!(self, Self::WordPressHook) + | Self::TwigBlock + | Self::TwigFilter + | Self::TwigFunction + | Self::TwigVariable => KeyFramework::Twig, + Self::YiiView | Self::YiiRoute | Self::YiiRouteArgument => KeyFramework::Yii, + Self::WordPressHook => KeyFramework::WordPress, + } } /// The bit a [`LookupGuards`] mask holds this kind under: the kinds a @@ -299,6 +294,25 @@ pub(crate) fn declares_another_name(kept: &SymbolSpan, next: &SymbolSpan) -> boo ) } +/// The framework a [`StringKeyKind`] belongs to, which is what decides +/// whether a project reads keys of that kind at all. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum KeyFramework { + Laravel, + /// Symfony's service IDs and container parameters, which any project + /// that builds a Symfony service container declares. + ServiceContainer, + /// The rest of Symfony: routes, translations, bundle configuration, + /// events, and Messenger. + Symfony, + /// Twig's template, block, filter, function, and variable names, + /// read in any project that uses Twig. + Twig, + Yii, + /// WordPress's hook names, read wherever the hook functions are called. + WordPress, +} + /// A workspace symbol whose presence suppresses an otherwise-valid framework /// runtime fallback. /// diff --git a/src/symfony/bundles.rs b/src/symfony/bundles.rs index b9c5e039c..f94a2d1fc 100644 --- a/src/symfony/bundles.rs +++ b/src/symfony/bundles.rs @@ -181,7 +181,16 @@ fn read_php_file(uri: &str, content: &str) -> SymbolMap { 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( - program, content, &names, true, false, None, + program, + content, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: true, + service_container: true, + drupal: false, + twig: false, + }, + None, ); let mut configurators = HashMap::new(); for statement in program.statements.iter() { diff --git a/src/symfony/mod.rs b/src/symfony/mod.rs index 0209df599..cef959974 100644 --- a/src/symfony/mod.rs +++ b/src/symfony/mod.rs @@ -48,7 +48,7 @@ use parking_lot::RwLock; use tower_lsp_server::gen_lsp_types::{Location, Range}; use crate::Backend; -use crate::symbol_map::{StringKeyKind, SymbolKind, SymbolMap, SymbolSpan}; +use crate::symbol_map::{KeyFramework, StringKeyKind, SymbolKind, SymbolMap, SymbolSpan}; use crate::text_position::LineIndex; /// The two formats Symfony's configuration is read from as text. @@ -135,8 +135,12 @@ pub(crate) struct SymfonyState { /// counted whether it has a name or not, for the arguments its handler /// reads from its path. fn declares_symfony_keys(map: &SymbolMap, enabled: impl Fn(StringKeyKind) -> bool) -> bool { - let counts = |kind: StringKeyKind| { - (kind.is_symfony() || kind.is_yii() || kind.is_twig()) && enabled(kind) + let counts = |kind: StringKeyKind| match kind.framework() { + KeyFramework::ServiceContainer + | KeyFramework::Symfony + | KeyFramework::Twig + | KeyFramework::Yii => enabled(kind), + KeyFramework::Laravel | KeyFramework::WordPress => false, }; (!map.messenger_handlers.is_empty() && enabled(StringKeyKind::MessengerBus)) || (!map.routes.is_empty() && enabled(StringKeyKind::YiiRoute)) @@ -174,19 +178,16 @@ impl Backend { /// Whether the framework a string key of `kind` belongs to is the one /// this project uses. pub(crate) fn string_key_kind_enabled(&self, kind: StringKeyKind) -> bool { - if kind.is_twig() { - self.is_twig_project() - } else if kind.is_wordpress() { + match kind.framework() { + KeyFramework::Laravel => self.resolved_class_cache.read().laravel().is_enabled(), + KeyFramework::ServiceContainer => self.reads_service_container(), + KeyFramework::Symfony => self.is_symfony_project(), + KeyFramework::Twig => self.is_twig_project(), + KeyFramework::Yii => self.is_yii_project(), // The hook functions' names are WordPress's own, and a project // that calls them is a WordPress one whatever its // `composer.json` says, or whether it has one at all. - true - } else if kind.is_yii() { - self.is_yii_project() - } else if kind.is_symfony() { - self.is_symfony_project() - } else { - self.resolved_class_cache.read().laravel().is_enabled() + KeyFramework::WordPress => true, } } @@ -195,7 +196,7 @@ impl Backend { /// Dependencies are never part of the project's own configuration, so a /// vendor file that happens to be parsed is left out. pub(crate) fn note_symfony_declarations(&self, uri: &str, map: &SymbolMap) { - if !self.is_symfony_project() && !self.is_twig_project() && !self.is_yii_project() { + if !self.reads_service_container() && !self.is_twig_project() && !self.is_yii_project() { return; } let declares = declares_symfony_keys(map, |kind| self.string_key_kind_enabled(kind)) @@ -272,11 +273,25 @@ 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. + let needs_typed = match kind.framework() { + KeyFramework::ServiceContainer => self.is_symfony_project(), + KeyFramework::Laravel + | KeyFramework::Symfony + | KeyFramework::Twig + | KeyFramework::Yii + | KeyFramework::WordPress => true, + }; for uri in uris { let Some(map) = self.symbol_map_for(&uri) else { continue; }; - let typed = self.typed_receiver_spans_for(&uri, &map); + let typed = if needs_typed { + self.typed_receiver_spans_for(&uri, &map) + } else { + std::sync::Arc::default() + }; for span in map.spans.iter().chain(typed.iter()) { if let SymbolKind::StringKey { kind: span_kind, diff --git a/src/symfony/route_world.rs b/src/symfony/route_world.rs index 7d588f508..e96db61b9 100644 --- a/src/symfony/route_world.rs +++ b/src/symfony/route_world.rs @@ -203,7 +203,16 @@ fn read_routing_file(path: &Path, content: &str) -> Option { 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( - program, content, &names, true, false, None, + program, + content, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: true, + service_container: true, + drupal: false, + twig: false, + }, + None, ); let mut spans: Vec<_> = map .spans diff --git a/src/twig/callables.rs b/src/twig/callables.rs index 1bcffd857..fb6658f86 100644 --- a/src/twig/callables.rs +++ b/src/twig/callables.rs @@ -141,7 +141,16 @@ fn read_extension_file(uri: &str, content: &str) -> Vec { 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( - program, content, &names, false, true, None, + program, + content, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: true, + }, + None, ); map.twig_callables .into_iter() diff --git a/src/type_engine/call_resolution/return_types/call_arms.rs b/src/type_engine/call_resolution/return_types/call_arms.rs index 82e0a0ff0..b8519c227 100644 --- a/src/type_engine/call_resolution/return_types/call_arms.rs +++ b/src/type_engine/call_resolution/return_types/call_arms.rs @@ -316,6 +316,24 @@ impl Backend { } let owner_class = all_owners.into_iter().next(); + // `\Drupal::service('id')` is typed by the class the ID is built + // from (see `crate::drupal::types`). + if method_name.eq_ignore_ascii_case("service") + && let Some(ref owner) = owner_class + && let Some(backend) = ctx.file.backend + && backend.is_drupal_project() + && let Some(id) = first_literal_text_arg(text_args, "id") + && let Some(classes) = crate::drupal::types::service_classes( + backend, + &owner.fqn(), + method_name, + id, + ctx.file.class_loader, + ) + { + return classes; + } + if let Some(ref owner) = owner_class { // A static call through a Laravel facade is typed by the // container class the facade forwards to, so that @@ -1090,3 +1108,11 @@ fn method_call_return_hint( ), ) } + +/// The value of the argument that binds a call's first parameter, named +/// `name`, when it is a plain string literal. +fn first_literal_text_arg<'t>(text_args: &'t str, name: &str) -> Option<&'t str> { + let args = split_top_level_commas(text_args); + let first = crate::call_args::first_param_text_arg(&args, name)?; + crate::text_scan::unquote_php_string(first.trim()) +} diff --git a/src/type_engine/variable/rhs_resolution/static_calls.rs b/src/type_engine/variable/rhs_resolution/static_calls.rs index 868870277..613fe3a3e 100644 --- a/src/type_engine/variable/rhs_resolution/static_calls.rs +++ b/src/type_engine/variable/rhs_resolution/static_calls.rs @@ -178,6 +178,14 @@ fn static_call_on_class( { return result; } + if let Some(result) = super::symfony_calls::try_resolve_drupal_service_type( + &owner.fqn(), + method_name, + argument_list, + ctx, + ) { + return result; + } } let arg_texts = static_arg_texts(static_call, ctx); diff --git a/src/type_engine/variable/rhs_resolution/symfony_calls.rs b/src/type_engine/variable/rhs_resolution/symfony_calls.rs index 3b1aa4d12..c5a28423a 100644 --- a/src/type_engine/variable/rhs_resolution/symfony_calls.rs +++ b/src/type_engine/variable/rhs_resolution/symfony_calls.rs @@ -37,7 +37,7 @@ pub(super) fn try_resolve_container_lookup_type( let backend = ctx .file .backend - .filter(|backend| backend.is_symfony_project())?; + .filter(|backend| backend.reads_service_container())?; let Some(Argument::Positional(argument)) = argument_list.arguments.first() else { return None; }; @@ -67,6 +67,37 @@ pub(super) fn try_resolve_container_lookup_type( } } +/// What `\Drupal::service()` returns for the service ID its literal first +/// argument names: the class the ID is built from, as for a container's +/// `get()`. +/// +/// `None` for an ID the project does not declare, and for a class constant +/// (`Foo::class`), which core's own conditional return type already reads. +pub(super) fn try_resolve_drupal_service_type( + owner_fqn: &str, + method_name: &str, + argument_list: &ArgumentList<'_>, + ctx: &VarResolutionCtx<'_>, +) -> Option> { + if !method_name.eq_ignore_ascii_case("service") { + return None; + } + let backend = ctx.file.backend?; + let argument = crate::call_args::first_param_arg(argument_list, "id")?; + let (key, _, _) = crate::virtual_members::laravel::helpers::resolved_string_literal( + argument.value(), + ctx.file.content, + )?; + let classes = crate::drupal::types::service_classes( + backend, + owner_fqn, + method_name, + key, + ctx.file.class_loader, + )?; + Some(ResolvedType::from_classes(classes)) +} + /// What Messenger's `HandleTrait::handle()` returns for the message it is /// given: what the message's handler returns, when exactly one handler is /// registered for it. `handle()` throws for a message with none or with diff --git a/src/wordpress/hook_sources.rs b/src/wordpress/hook_sources.rs index 6c6da4c35..e161db471 100644 --- a/src/wordpress/hook_sources.rs +++ b/src/wordpress/hook_sources.rs @@ -110,7 +110,16 @@ pub(crate) fn hook_firings(content: &str) -> Vec { 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( - program, content, &names, false, false, None, + program, + content, + &names, + crate::symbol_map::extraction::Frameworks { + symfony: false, + service_container: false, + drupal: false, + twig: false, + }, + None, ); map.spans .into_iter() diff --git a/src/workspace_env.rs b/src/workspace_env.rs index 5da7722fe..b2bb9b6c0 100644 --- a/src/workspace_env.rs +++ b/src/workspace_env.rs @@ -96,7 +96,9 @@ pub(crate) struct WorkspaceEnv { pub(crate) client_indexing: Arc>, /// Whether a Composer project of the workspace is a Drupal site, so /// `index_filters` treats Drupal's `.module`, `.install`, … files as - /// PHP source in every walk, not only in the web-root class scan. + /// PHP source in every walk, not only in the web-root class scan, and + /// Drupal's own declarations, such as its services, are read (see + /// [`crate::drupal`]). pub(crate) is_drupal: Arc, /// The stub directory the editor extension forwarded, beside the /// `[stubs]` paths of `config`. Shared by `Arc` for the same reason diff --git a/tests/integration/drupal_mixed_workspace.rs b/tests/integration/drupal_mixed_workspace.rs new file mode 100644 index 000000000..4f6fb88ac --- /dev/null +++ b/tests/integration/drupal_mixed_workspace.rs @@ -0,0 +1,590 @@ +//! A Drupal site in a workspace with other frameworks' projects: each of +//! them keeps working in its own project, and Drupal works in its site. +//! +//! The Laravel application is the workspace root, since Laravel's +//! configuration and routes are read from there, and the Drupal site, the +//! Symfony application, and the WordPress plugin are Composer projects +//! installed beside it. The Drupal flag is the workspace's, as every +//! framework's is, so only what works is checked here; what a project +//! without Drupal does not get is checked in a workspace of its own. + +use crate::common::{ + completion_response, hover_text_at, initialize_with_options, offset_position, open_document, + response_labels, +}; +use phpantom_lsp::Backend; +use tower_lsp_server::LanguageServer; +use tower_lsp_server::gen_lsp_types::*; + +// ─── Laravel, at the root ─────────────────────────────────────────────────── + +const LARAVEL_COMPOSER: &str = r#"{ + "require": { "laravel/framework": "^11.0" }, + "autoload": { "psr-4": { "App\\": "app/" } } +}"#; + +const LARAVEL_CONFIG: &str = " 'Acme',\n];\n"; + +const LARAVEL_SETTINGS: &str = r#"services(); + $services->set('app.php_mailer', \Shop\Service\Mailer::class); +}; +"#; + +const SYMFONY_CONSUMER: &str = r#"get('app.mailer'); + $mailer; + $container->get('app.php_mailer'); + $container->get('app.'); + } +} +"#; + +const SYMFONY_YAML_PATH: &str = "symfony/config/services.yaml"; +const SYMFONY_PHP_PATH: &str = "symfony/config/services.php"; +const SYMFONY_CONSUMER_PATH: &str = "symfony/src/Controller/MailController.php"; + +// ─── WordPress, in `wordpress-plugin/` ────────────────────────────────────── + +const WORDPRESS_COMPOSER: &str = r#"{ + "autoload": { "psr-4": { "Acme\\Plugin\\": "src/" } } +}"#; + +const WORDPRESS_FIRES: &str = r#" Vec<(String, &'static str)> { + let mut files: Vec<(String, &'static str)> = vec![ + ("composer.json".to_string(), LARAVEL_COMPOSER), + ("config/app.php".to_string(), LARAVEL_CONFIG), + ("app/Settings.php".to_string(), LARAVEL_SETTINGS), + ("drupal/composer.json".to_string(), DRUPAL_COMPOSER), + ( + "drupal/vendor/composer/installed.json".to_string(), + INSTALLED, + ), + ("drupal/web/core/lib/Drupal.php".to_string(), DRUPAL_CLASS), + ( + "drupal/web/modules/custom/acme/src/Greeter.php".to_string(), + DRUPAL_GREETER, + ), + (DRUPAL_SERVICES_PATH.to_string(), DRUPAL_SERVICES), + (DRUPAL_CONSUMER_PATH.to_string(), DRUPAL_CONSUMER), + ("symfony/composer.json".to_string(), SYMFONY_COMPOSER), + ( + "symfony/vendor/composer/installed.json".to_string(), + INSTALLED, + ), + (SYMFONY_YAML_PATH.to_string(), SYMFONY_SERVICES_YAML), + (SYMFONY_PHP_PATH.to_string(), SYMFONY_SERVICES_PHP), + (SYMFONY_CONSUMER_PATH.to_string(), SYMFONY_CONSUMER), + ( + "wordpress-plugin/composer.json".to_string(), + WORDPRESS_COMPOSER, + ), + ( + "wordpress-plugin/vendor/composer/installed.json".to_string(), + INSTALLED, + ), + (WORDPRESS_FIRES_PATH.to_string(), WORDPRESS_FIRES), + (WORDPRESS_LISTENS_PATH.to_string(), WORDPRESS_LISTENS), + ]; + files.extend( + SYMFONY_STUBS + .iter() + .map(|(path, content)| (format!("symfony/{path}"), *content)), + ); + files +} + +/// An initialized workspace holding `files`, with every PHP and YAML file +/// among them open. +async fn workspace(files: &[(String, &str)]) -> (Backend, tempfile::TempDir) { + let dir = tempfile::tempdir().unwrap(); + for (path, content) in 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(); + initialize_with_options(&backend, dir.path(), None).await; + for (path, content) in files { + let language = if path.ends_with(".php") { + "php" + } else if path.ends_with(".yml") || path.ends_with(".yaml") { + "yaml" + } else { + continue; + }; + open_document(&backend, &uri_for(&dir, path), language, content).await; + } + (backend, dir) +} + +fn uri_for(dir: &tempfile::TempDir, rel: &str) -> Uri { + Uri::from_file_path(dir.path().join(rel)).unwrap() +} + +/// The position `inside` bytes into the first occurrence of `needle`. +fn position_in(content: &str, needle: &str, inside: usize) -> Position { + offset_position( + content, + content.find(needle).expect("needle should exist") + inside, + ) +} + +/// The 0-based line of the first occurrence of `needle`. +fn line_of(content: &str, needle: &str) -> u32 { + position_in(content, needle, 0).line +} + +/// The definitions of the name at `position`, as files and lines. +async fn definitions(backend: &Backend, uri: &Uri, position: Position) -> Vec<(Uri, u32)> { + let response = backend + .goto_definition(DefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }) + .await + .unwrap(); + let locations = match response { + None => Vec::new(), + Some(DefinitionResponse::Definition(Definition::Location(location))) => vec![location], + Some(DefinitionResponse::Definition(Definition::LocationList(locations))) => locations, + Some(DefinitionResponse::DefinitionLinkList(_)) => panic!("unexpected location links"), + }; + locations + .into_iter() + .map(|location| (location.uri, location.range.start.line)) + .collect() +} + +/// Every reference to the name at `position`, declarations included, as +/// files and lines. +async fn references(backend: &Backend, uri: &Uri, position: Position) -> Vec<(Uri, u32)> { + let mut found: Vec<(Uri, u32)> = backend + .references(ReferenceParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + context: ReferenceContext { + include_declaration: true, + }, + }) + .await + .unwrap() + .unwrap_or_default() + .into_iter() + .map(|location| (location.uri, location.range.start.line)) + .collect(); + found.sort(); + found +} + +async fn completion_labels(backend: &Backend, uri: &Uri, position: Position) -> Vec { + response_labels( + completion_response(backend, uri, position.line, position.character, None).await, + ) +} + +async fn hover_at(backend: &Backend, uri: &Uri, position: Position) -> String { + hover_text_at(backend, uri, position.line, position.character) + .await + .unwrap_or_default() +} + +#[tokio::test] +async fn symfony_services_keep_working_beside_a_drupal_site() { + let (backend, dir) = workspace(&mixed_files()).await; + let consumer = uri_for(&dir, SYMFONY_CONSUMER_PATH); + + let found = definitions( + &backend, + &consumer, + position_in(SYMFONY_CONSUMER, "get('app.mailer')", 7), + ) + .await; + assert_eq!( + found, + vec![( + uri_for(&dir, SYMFONY_YAML_PATH), + line_of(SYMFONY_SERVICES_YAML, "app.mailer:") + )] + ); + + let found = definitions( + &backend, + &consumer, + position_in(SYMFONY_CONSUMER, "get('app.php_mailer')", 7), + ) + .await; + assert_eq!( + found, + vec![( + uri_for(&dir, SYMFONY_PHP_PATH), + line_of(SYMFONY_SERVICES_PHP, "set('app.php_mailer'") + )] + ); + + let hover = hover_at( + &backend, + &consumer, + position_in(SYMFONY_CONSUMER, "$mailer;", 2), + ) + .await; + assert!(hover.contains("Mailer"), "{hover}"); + + let labels = completion_labels( + &backend, + &consumer, + position_in(SYMFONY_CONSUMER, "get('app.')", 9), + ) + .await; + for id in ["app.mailer", "app.php_mailer"] { + assert!(labels.contains(&id.to_string()), "{id}: {labels:?}"); + } +} + +#[tokio::test] +async fn laravel_config_keys_keep_working_beside_a_drupal_site() { + let (backend, dir) = workspace(&mixed_files()).await; + let found = definitions( + &backend, + &uri_for(&dir, "app/Settings.php"), + position_in(LARAVEL_SETTINGS, "config('app.name')", 10), + ) + .await; + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!(found[0].0, uri_for(&dir, "config/app.php")); +} + +#[tokio::test] +async fn wordpress_hooks_keep_working_beside_a_drupal_site() { + let (backend, dir) = workspace(&mixed_files()).await; + let found = definitions( + &backend, + &uri_for(&dir, WORDPRESS_LISTENS_PATH), + position_in(WORDPRESS_LISTENS, "'acme_order_placed'", 3), + ) + .await; + assert_eq!( + found, + vec![( + uri_for(&dir, WORDPRESS_FIRES_PATH), + line_of(WORDPRESS_FIRES, "do_action(") + )] + ); +} + +#[tokio::test] +async fn drupal_services_work_in_the_drupal_site() { + let (backend, dir) = workspace(&mixed_files()).await; + let consumer = uri_for(&dir, DRUPAL_CONSUMER_PATH); + + let hover = hover_at( + &backend, + &consumer, + position_in(DRUPAL_CONSUMER, "$greeter;", 2), + ) + .await; + assert!(hover.contains("Greeter"), "{hover}"); + + let found = definitions( + &backend, + &consumer, + position_in(DRUPAL_CONSUMER, "service('acme.greeter')", 10), + ) + .await; + assert_eq!( + found, + vec![( + uri_for(&dir, DRUPAL_SERVICES_PATH), + line_of(DRUPAL_SERVICES, "acme.greeter:") + )] + ); +} + +/// 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 +/// classes, which leave Symfony's configurator out. +#[tokio::test] +async fn a_drupal_lookup_leaves_the_symfony_configurator_readable() { + let dir = tempfile::tempdir().unwrap(); + let files = mixed_files(); + for (path, content) in &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(); + initialize_with_options(&backend, dir.path(), None).await; + let drupal_services = uri_for(&dir, DRUPAL_SERVICES_PATH); + open_document(&backend, &drupal_services, "yaml", DRUPAL_SERVICES).await; + let drupal_consumer = uri_for(&dir, DRUPAL_CONSUMER_PATH); + open_document(&backend, &drupal_consumer, "php", DRUPAL_CONSUMER).await; + let symfony_config = uri_for(&dir, SYMFONY_PHP_PATH); + open_document(&backend, &symfony_config, "php", SYMFONY_SERVICES_PHP).await; + + let hover = hover_at( + &backend, + &drupal_consumer, + position_in(DRUPAL_CONSUMER, "$greeter;", 2), + ) + .await; + assert!(hover.contains("Greeter"), "{hover}"); + + let found = definitions( + &backend, + &symfony_config, + position_in(SYMFONY_SERVICES_PHP, "'app.php_mailer'", 3), + ) + .await; + assert_eq!( + found, + vec![( + symfony_config.clone(), + line_of(SYMFONY_SERVICES_PHP, "set('app.php_mailer'") + )] + ); +} + +// ─── A Symfony application alone ─────────────────────────────────────────── + +/// The class `\Drupal` a Symfony application may hold without being a +/// Drupal site: a library's, or one of its own. +const LONE_DRUPAL_CLASS: &str = + "get('app.mailer'); + $mailer; + $other = \Drupal::service('app.mailer'); + $other; + \Drupal::service('app.'); + } +} +"#; + +const LONE_CONSUMER_PATH: &str = "src/Controller/Lookups.php"; + +/// A workspace that is a Symfony application and nothing else. +async fn symfony_alone() -> (Backend, tempfile::TempDir) { + let mut files: Vec<(String, &str)> = vec![ + ("composer.json".to_string(), SYMFONY_COMPOSER), + ("config/services.yaml".to_string(), SYMFONY_SERVICES_YAML), + ("stubs/Drupal.php".to_string(), LONE_DRUPAL_CLASS), + (LONE_CONSUMER_PATH.to_string(), LONE_CONSUMER), + ]; + files.extend( + SYMFONY_STUBS + .iter() + .map(|(path, content)| (path.to_string(), *content)), + ); + workspace(&files).await +} + +/// Outside a Drupal site, `\Drupal::service()` names no service: nothing +/// completes in it, it is no reference to a service, and it keeps the +/// return type its class declares. +#[tokio::test] +async fn drupal_service_names_nothing_in_a_symfony_application_alone() { + let (backend, dir) = symfony_alone().await; + let consumer = uri_for(&dir, LONE_CONSUMER_PATH); + + let labels = completion_labels( + &backend, + &consumer, + position_in(LONE_CONSUMER, "service('app.')", 13), + ) + .await; + assert!(!labels.contains(&"app.mailer".to_string()), "{labels:?}"); + + let found = definitions( + &backend, + &consumer, + position_in(LONE_CONSUMER, "service('app.mailer')", 10), + ) + .await; + assert!(found.is_empty(), "{found:?}"); + + // The container lookup is a reference; `\Drupal::service()` is not. + let found = references( + &backend, + &uri_for(&dir, "config/services.yaml"), + position_in(SYMFONY_SERVICES_YAML, "app.mailer:", 2), + ) + .await; + assert!( + found.contains(&( + consumer.clone(), + line_of(LONE_CONSUMER, "get('app.mailer')") + )), + "{found:?}" + ); + assert!( + !found.contains(&( + consumer.clone(), + line_of(LONE_CONSUMER, "service('app.mailer')") + )), + "{found:?}" + ); + + let hover = hover_at( + &backend, + &consumer, + position_in(LONE_CONSUMER, "$mailer;", 2), + ) + .await; + assert!(hover.contains("Mailer"), "{hover}"); + let hover = hover_at( + &backend, + &consumer, + position_in(LONE_CONSUMER, "$other;", 2), + ) + .await; + assert!(!hover.contains("Mailer"), "{hover}"); + assert!(hover.contains("object"), "{hover}"); +} diff --git a/tests/integration/drupal_services.rs b/tests/integration/drupal_services.rs new file mode 100644 index 000000000..55e1e508c --- /dev/null +++ b/tests/integration/drupal_services.rs @@ -0,0 +1,577 @@ +//! Drupal's services: the IDs a module's, core's, or the site's +//! `*.services.yml` declares, read like Symfony's container configuration +//! but found where Drupal keeps them. + +use crate::common::{ + completion_response, hover_text_at, initialize_with_options, offset_position, open_document, + response_labels, +}; +use phpantom_lsp::Backend; +use tower_lsp_server::LanguageServer; +use tower_lsp_server::gen_lsp_types::*; + +const COMPOSER: &str = r#"{ + "require": { "drupal/core-recommended": "^11" }, + "extra": { "drupal-scaffold": { "locations": { "web-root": "web/" } } } +}"#; + +/// Just enough of the container interfaces, where Composer would put core. +const STUBS: &[(&str, &str)] = &[ + ( + "web/core/lib/Psr/Container/ContainerInterface.php", + "get('acme.greeter'); + $greeter; + } +} +"#; + +const SERVICES_PATH: &str = "web/modules/custom/acme/acme.services.yml"; +const CONSUMER_PATH: &str = "web/modules/custom/acme/src/Consumer.php"; + +/// An initialized Drupal project holding the stubs and `files`, every one +/// of `files` open. +async fn workspace(files: &[(&str, &str)]) -> (Backend, tempfile::TempDir) { + workspace_in(tempfile::tempdir().unwrap(), files).await +} + +/// [`workspace`] in `dir`, which may hold files already. +async fn workspace_in( + dir: tempfile::TempDir, + files: &[(&str, &str)], +) -> (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(); + initialize_with_options(&backend, dir.path(), None).await; + for (path, content) in files { + let language = if path.ends_with(".php") { + "php" + } else { + "yaml" + }; + open_document(&backend, &uri_for(&dir, path), language, content).await; + } + (backend, dir) +} + +fn uri_for(dir: &tempfile::TempDir, rel: &str) -> Uri { + Uri::from_file_path(dir.path().join(rel)).unwrap() +} + +/// The position `inside` bytes into the first occurrence of `needle`. +fn position_in(content: &str, needle: &str, inside: usize) -> Position { + offset_position( + content, + content.find(needle).expect("needle should exist") + inside, + ) +} + +#[tokio::test] +async fn a_container_lookup_is_typed_by_the_declared_class() { + let (backend, dir) = workspace(&[(SERVICES_PATH, SERVICES), (CONSUMER_PATH, CONSUMER)]).await; + let position = position_in(CONSUMER, "$greeter;", 2); + let hover = hover_text_at( + &backend, + &uri_for(&dir, CONSUMER_PATH), + position.line, + position.character, + ) + .await + .unwrap_or_default(); + assert!(hover.contains("Greeter"), "{hover}"); +} + +#[tokio::test] +async fn a_container_lookup_goes_to_the_services_file() { + let (backend, dir) = workspace(&[(SERVICES_PATH, SERVICES), (CONSUMER_PATH, CONSUMER)]).await; + let response = backend + .goto_definition(DefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: uri_for(&dir, CONSUMER_PATH), + }, + position: position_in(CONSUMER, "get('acme.greeter')", 7), + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }) + .await + .unwrap(); + let locations = match response { + Some(DefinitionResponse::Definition(Definition::Location(location))) => vec![location], + Some(DefinitionResponse::Definition(Definition::LocationList(locations))) => locations, + other => panic!("unexpected {other:?}"), + }; + assert_eq!(locations.len(), 1, "{locations:?}"); + assert_eq!(locations[0].uri, uri_for(&dir, SERVICES_PATH)); + assert_eq!(locations[0].range.start.line, 1); +} + +#[tokio::test] +async fn service_ids_complete_in_php_and_in_the_services_file() { + let consumer = CONSUMER.replace("get('acme.greeter')", "get('acme.')"); + let yaml = format!("{SERVICES} acme.other:\n arguments: ['@acme.']\n"); + let (backend, dir) = workspace(&[ + (SERVICES_PATH, yaml.as_str()), + (CONSUMER_PATH, consumer.as_str()), + ]) + .await; + + let position = position_in(&consumer, "get('acme.')", 10); + let labels = response_labels( + completion_response( + &backend, + &uri_for(&dir, CONSUMER_PATH), + position.line, + position.character, + None, + ) + .await, + ); + assert!(labels.contains(&"acme.greeter".to_string()), "{labels:?}"); + + let position = position_in(&yaml, "'@acme.']", 7); + let labels = response_labels( + completion_response( + &backend, + &uri_for(&dir, SERVICES_PATH), + position.line, + position.character, + None, + ) + .await, + ); + assert!(labels.contains(&"acme.greeter".to_string()), "{labels:?}"); +} + +/// Every reference to the name at `position`, declarations included, as +/// files and lines. +async fn references_at(backend: &Backend, uri: &Uri, position: Position) -> Vec<(Uri, u32)> { + let mut found: Vec<(Uri, u32)> = backend + .references(ReferenceParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + context: ReferenceContext { + include_declaration: true, + }, + }) + .await + .unwrap() + .unwrap_or_default() + .into_iter() + .map(|location| (location.uri, location.range.start.line)) + .collect(); + found.sort(); + found +} + +#[tokio::test] +async fn references_reach_the_lookup_and_the_yaml_argument() { + let (backend, dir) = workspace(&[(SERVICES_PATH, SERVICES), (CONSUMER_PATH, CONSUMER)]).await; + let found = references_at( + &backend, + &uri_for(&dir, SERVICES_PATH), + position_in(SERVICES, "acme.greeter:", 3), + ) + .await; + let yaml = uri_for(&dir, SERVICES_PATH); + let php = uri_for(&dir, CONSUMER_PATH); + let mut expected = vec![(yaml.clone(), 1), (yaml, 4), (php, 7)]; + expected.sort(); + assert_eq!(found, expected); +} + +/// Core sits in a folder the project's `.gitignore` leaves out, and its +/// test modules declare services no real site has. +#[tokio::test] +async fn core_services_are_read_and_test_modules_are_not() { + let core_services = + "services:\n config.factory:\n class: Drupal\\Core\\Config\\ConfigFactory\n"; + let fixture_services = + "services:\n config.fixture:\n class: Drupal\\Core\\Config\\ConfigFactory\n"; + let consumer = CONSUMER.replace( + "$greeter = $container->get('acme.greeter');", + "$greeter = $container->get('config.factory');\n $container->get('config.fixture');", + ); + let dir = tempfile::tempdir().unwrap(); + for (path, content) in [ + // A repository whose `.gitignore` leaves core out, as a Composer + // install of Drupal does. + (".git/HEAD", "ref: refs/heads/main\n"), + (".gitignore", "/web/core/\n"), + ("web/core/core.services.yml", core_services), + ( + "web/core/modules/system/tests/modules/fixture/fixture.services.yml", + fixture_services, + ), + ( + "web/core/lib/Drupal/Core/Config/ConfigFactory.php", + " Vec { + let response = backend + .goto_definition(DefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }) + .await + .unwrap(); + match response { + None => Vec::new(), + Some(DefinitionResponse::Definition(Definition::Location(location))) => vec![location], + Some(DefinitionResponse::Definition(Definition::LocationList(locations))) => locations, + Some(DefinitionResponse::DefinitionLinkList(_)) => panic!("unexpected location links"), + } +} + +#[tokio::test] +async fn drupal_service_is_typed_and_navigates_like_a_container_lookup() { + let (backend, dir) = workspace(&[ + (SERVICES_PATH, SERVICES), + (STATIC_CONSUMER_PATH, STATIC_CONSUMER), + ]) + .await; + let php = uri_for(&dir, STATIC_CONSUMER_PATH); + + let position = position_in(STATIC_CONSUMER, "$greeter;", 2); + let hover = hover_text_at(&backend, &php, position.line, position.character) + .await + .unwrap_or_default(); + assert!(hover.contains("Greeter"), "{hover}"); + + for needle in ["service('acme.greeter')", "hasService('acme.greeter')"] { + let inside = needle.find('\'').unwrap() + 2; + let found = + definitions_at(&backend, &php, position_in(STATIC_CONSUMER, needle, inside)).await; + assert_eq!(found.len(), 1, "{needle}: {found:?}"); + assert_eq!(found[0].uri, uri_for(&dir, SERVICES_PATH)); + } +} + +/// A file the editor opens before the workspace walk finds the Drupal site +/// is read again once it does. +#[tokio::test] +async fn a_file_opened_before_the_walk_ends_reads_drupal_names() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("composer.json"), COMPOSER).unwrap(); + for (path, content) in STUBS.iter().chain(&[ + (SERVICES_PATH, SERVICES), + (STATIC_CONSUMER_PATH, STATIC_CONSUMER), + ]) { + 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(); + let php = uri_for(&dir, STATIC_CONSUMER_PATH); + open_document(&backend, &php, "php", STATIC_CONSUMER).await; + backend.initialized(InitializedParams {}).await; + // A test backend runs no background index, which would read it. + open_document(&backend, &uri_for(&dir, SERVICES_PATH), "yaml", SERVICES).await; + + let found = definitions_at( + &backend, + &php, + position_in(STATIC_CONSUMER, "service('acme.greeter')", 10), + ) + .await; + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!(found[0].uri, uri_for(&dir, SERVICES_PATH)); +} + +#[tokio::test] +async fn drupal_service_ids_complete() { + let consumer = STATIC_CONSUMER.replace("service('acme.greeter')", "service('acme.')"); + let (backend, dir) = workspace(&[ + (SERVICES_PATH, SERVICES), + (STATIC_CONSUMER_PATH, consumer.as_str()), + ]) + .await; + let position = position_in(&consumer, "service('acme.')", 9); + let labels = response_labels( + completion_response( + &backend, + &uri_for(&dir, STATIC_CONSUMER_PATH), + position.line, + position.character, + None, + ) + .await, + ); + assert!(labels.contains(&"acme.greeter".to_string()), "{labels:?}"); +} + +#[tokio::test] +async fn a_drupal_class_in_another_namespace_names_no_service() { + let consumer = r#"greet();", + ); + let (backend, dir) = workspace(&[ + (SERVICES_PATH, SERVICES), + (STATIC_CONSUMER_PATH, chained.as_str()), + ]) + .await; + let found = definitions_at( + &backend, + &uri_for(&dir, STATIC_CONSUMER_PATH), + position_in(&chained, "->greet()", 3), + ) + .await; + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!( + found[0].uri, + uri_for(&dir, "web/modules/custom/acme/src/Greeter.php") + ); +} + +/// `\Drupal::service(id: 'x')` names the same service as the positional +/// form, assigned or chained. +#[tokio::test] +async fn a_named_service_id_is_typed() { + let named = STATIC_CONSUMER.replace("service('acme.greeter')", "service(id: 'acme.greeter')"); + let (backend, dir) = workspace(&[ + (SERVICES_PATH, SERVICES), + (STATIC_CONSUMER_PATH, named.as_str()), + ]) + .await; + let position = position_in(&named, "$greeter;", 2); + let hover = hover_text_at( + &backend, + &uri_for(&dir, STATIC_CONSUMER_PATH), + position.line, + position.character, + ) + .await + .unwrap_or_default(); + assert!(hover.contains("Greeter"), "{hover}"); +} + +/// Core's `\Drupal::service()`, with the conditional return type that types +/// a lookup by class name as that class. +const DRUPAL_WITH_CONDITIONAL_RETURN: &str = r#"|string $id + * + * @template T of object + * + * @return ($id is class-string ? T : object) + */ + public static function service(string $id): object {} +} +"#; + +const GREETER_INTERFACE: &str = " + } +} +"#; + +const INTERFACE_CONSUMER_PATH: &str = "web/modules/custom/acme/src/InterfaceConsumer.php"; + +/// A service registered under its interface is fetched by the interface's +/// class name, and typed as that interface rather than as the class the +/// alias leads to. +#[tokio::test] +async fn a_service_fetched_by_its_interface_is_typed_as_the_interface() { + let services = format!("{SERVICES} Drupal\\acme\\GreeterInterface: '@acme.greeter'\n"); + let (backend, dir) = workspace(&[ + ("web/core/lib/Drupal.php", DRUPAL_WITH_CONDITIONAL_RETURN), + ( + "web/modules/custom/acme/src/GreeterInterface.php", + GREETER_INTERFACE, + ), + (SERVICES_PATH, services.as_str()), + (INTERFACE_CONSUMER_PATH, INTERFACE_CONSUMER), + ]) + .await; + let consumer = uri_for(&dir, INTERFACE_CONSUMER_PATH); + + let position = position_in(INTERFACE_CONSUMER, "$greeter;", 2); + let hover = hover_text_at(&backend, &consumer, position.line, position.character) + .await + .unwrap_or_default(); + assert!(hover.contains("GreeterInterface"), "{hover}"); + + let needle = "GreeterInterface::class)->"; + let position = position_in(INTERFACE_CONSUMER, needle, needle.len()); + let labels = response_labels( + completion_response(&backend, &consumer, position.line, position.character, None).await, + ); + assert!( + labels + .iter() + .any(|label| label.starts_with("greetFormally")), + "{labels:?}" + ); +} diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 4d2fe582e..01c2ef528 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -200,6 +200,8 @@ mod document_highlight; mod document_links; mod document_symbols; mod document_symbols_blade; +mod drupal_mixed_workspace; +mod drupal_services; mod duplicate_class_declarations; mod duplicate_function_declarations; mod expression_evaluation_order;