diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 85a351e15..8c0032156 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1573,6 +1573,13 @@ when a selector changes. The dynamic wrapper's scalar attributes become strings, while objects and arrays enter the inner view's data. Literal `:is` names navigate to components; variables and constants keep PHP navigation. +Tempest route attributes keep their built-in prefixes precomputed. Custom +route decorators are resolved once per attribute class within a request, in +method-then-class order. Their `decorate()` bodies are read in the declaring +file's namespace, including inherited and trait methods; recognized literal +prefix transformations supply the same URI to completion, navigation, and +parameter references without copying the route's declaration data. + ## Tempest model property names Tempest database property arguments use the shared resolver to read the model diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a688d3f58..86be4bf71 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -176,6 +176,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Tempest +- **Custom route decorators.** Route completion and navigation follow URI prefixes added by a project's own decorators. Contributed by @shuvroroy. - **Queries built from model class names.** `query(Book::class)` keeps query results typed as `Book` objects through fluent calls. Contributed by @shuvroroy. - **Container invocation arguments.** The named arguments `Container::invoke()` and the `invoke()` helper forward complete from the callable's parameters and go to them. Contributed by @shuvroroy. - **Built-in view documentation.** Completion and hover explain Tempest's built-in components and directives, with links to the view specification. Contributed by @shuvroroy. diff --git a/docs/todo.md b/docs/todo.md index 7d86b1cbd..48f4e1218 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -158,7 +158,6 @@ ships, remove every row that names it. | # | Item | Impact | Complexity | | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- | | TM8 | [The data `view()` passes completes and navigates](todo/tempest.md#tm8-the-data-view-passes-completes-and-navigates) | Low-Medium | Low-Medium | -| TM7 | [A project's own route decorators change the route's URI](todo/tempest.md#tm7-a-projects-own-route-decorators-change-the-routes-uri) | Low | Medium | ## PHPUnit & Pest diff --git a/docs/todo/tempest.md b/docs/todo/tempest.md index da8eee715..0f94e8e82 100644 --- a/docs/todo/tempest.md +++ b/docs/todo/tempest.md @@ -5,8 +5,8 @@ templates already get completion, hover, go-to-definition, and diagnostics through virtual PHP, the data a view is rendered with is typed from the calls and View objects that render it, and component tags, their attributes, and the paths passed to `view()` complete and -navigate. What is still missing is route URIs that a project's own -decorators change. +navigate. The remaining gap is completing and navigating the data +passed to a view at the call site. Work out each behaviour from the framework's own source (`tempest/view`, `tempest/router`, `tempest/database`, @@ -44,36 +44,3 @@ template reads that the call does not pass yet, and navigate and hover from a name to the variable, reusing the reverse of the lookup that types the template (`src/tempest/view_data.rs`). A custom View object's constructor arguments are ordinary parameters and keep their handling. - ---- - -## TM7. A project's own route decorators change the route's URI - -**Impact: Low ยท Complexity: Medium** - -```php -#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD)] -final class Admin implements RouteDecorator -{ - public function decorate(Route $route): Route - { - return new Prefix('/admin')->decorate($route); - } -} - -#[Admin] -final class BookController -{ - #[Get('/books/{book}')] // the route is /admin/books/{book} - public function show(Book $book): View { /* ... */ } -} -``` - -Route discovery applies every attribute that implements -`Tempest\Router\RouteDecorator`, method ones before class ones, but only -`Tempest\Router\Prefix` is read for the URI it adds. A project's own -decorator that rewrites the URI is skipped, so its routes are recorded -without the prefix, and a `uri()` literal that includes it finds no -route. Read a decorator whose `decorate()` hands the route to a `Prefix` -it builds from a literal, or assigns `$route->uri` from a literal joined -with the old URI, as that prefix. diff --git a/examples/tempest/README.md b/examples/tempest/README.md index ccec2c8fd..3db61733b 100644 --- a/examples/tempest/README.md +++ b/examples/tempest/README.md @@ -13,7 +13,8 @@ The `.view.php` files use the editor's PHP language-server connection. In `BookController.php`, follow route placeholders to parameters and rename them together with their URI arguments. Routes without a leading slash complete as absolute paths. Custom route attributes and controller - prefixes work too. + prefixes work too. The custom `Admin` and `Versioned` decorators produce + `/admin/v1/books/{book}`; method decorators run before controller decorators. - In `app/books.view.php`, complete directives and component tags, follow `` to its file, and complete or navigate `$book`'s members. Follow `:title` and `:show-title` to their first variable reads in the diff --git a/examples/tempest/app/Admin.php b/examples/tempest/app/Admin.php new file mode 100644 index 000000000..3ddff8400 --- /dev/null +++ b/examples/tempest/app/Admin.php @@ -0,0 +1,19 @@ +decorate($route); + } +} diff --git a/examples/tempest/app/BookController.php b/examples/tempest/app/BookController.php index d7c328139..2d1fa08e9 100644 --- a/examples/tempest/app/BookController.php +++ b/examples/tempest/app/BookController.php @@ -9,6 +9,7 @@ use function Tempest\View\view; +#[Admin] final class BookController { #[Get('/books')] @@ -19,6 +20,8 @@ public function index(): View } // Try: complete inside {book}, or follow it to the parameter below. + // The method decorator runs first: /admin/v1/books/{book}. + #[Versioned] #[ReadBook('books/{book}')] public function show(string $book): string { diff --git a/examples/tempest/app/RouteLinks.php b/examples/tempest/app/RouteLinks.php index 887f42c8d..832144129 100644 --- a/examples/tempest/app/RouteLinks.php +++ b/examples/tempest/app/RouteLinks.php @@ -14,8 +14,8 @@ public function links(): array return [ // Try: complete book:, follow it to $book, or rename the parameter. uri([BookController::class, 'show'], book: 'php'), - // Try: complete the URI string or follow it to show(). - uri('/books/{book}', book: 'php'), + // Try: complete the decorated URI or follow it to show(). + uri('/admin/v1/books/{book}', book: 'php'), // Try: complete locale: or follow it to the invokable action's parameter. uri(HomeController::class, locale: 'en'), // Try: complete a controller name or follow it to __invoke(). diff --git a/examples/tempest/app/Versioned.php b/examples/tempest/app/Versioned.php new file mode 100644 index 000000000..410f1ef4e --- /dev/null +++ b/examples/tempest/app/Versioned.php @@ -0,0 +1,20 @@ +uri = '/v1/' . $route->uri; + + return $route; + } +} diff --git a/examples/tempest/assertions.php b/examples/tempest/assertions.php index 979a839d1..07a2ab336 100644 --- a/examples/tempest/assertions.php +++ b/examples/tempest/assertions.php @@ -64,8 +64,8 @@ ); $container->singleton(Tempest\Router\UriGenerator::class, $generator); if (new App\RouteLinks()->links() !== [ - 'https://books.example/books/php', - 'https://books.example/books/php', + 'https://books.example/admin/v1/books/php', + 'https://books.example/admin/v1/books/php', 'https://books.example/api/home/en', 'https://books.example/api/home/en', ]) { diff --git a/src/symbol_map/extraction/tempest.rs b/src/symbol_map/extraction/tempest.rs index 3e1885e46..005ee661f 100644 --- a/src/symbol_map/extraction/tempest.rs +++ b/src/symbol_map/extraction/tempest.rs @@ -7,7 +7,7 @@ use crate::virtual_members::laravel::helpers::resolved_string_literal_including_ /// Keep route-shaped attributes during the class walk. Their types are /// resolved later, when cross-file interfaces are available. pub(super) fn record_route_attributes(class: &Class<'_>, ctx: &mut ExtractionCtx<'_>) { - use crate::tempest::routes::{RouteAttribute, RouteParameter, placeholders}; + use crate::tempest::routes::{RouteAttribute, RouteDecorator, RouteParameter, placeholders}; if !ctx.tempest { return; @@ -62,7 +62,8 @@ pub(super) fn record_route_attributes(class: &Class<'_>, ctx: &mut ExtractionCtx offset: parameter.variable.span.start.offset, }) .collect(); - let mut route_path = path.to_string(); + let mut decorators = Vec::new(); + let mut route_path = path.to_owned(); let mut route_placeholders: Vec<_> = placeholders(&ctx.content[start..end]) .into_iter() .map(|(from, to)| (start as u32 + from as u32, start as u32 + to as u32)) @@ -73,10 +74,11 @@ pub(super) fn record_route_attributes(class: &Class<'_>, ctx: &mut ExtractionCtx .chain(class.attribute_lists.iter()) .flat_map(|list| list.attributes.iter()) { - if !ctx - .resolved_name_at(decorator.name.span().start.offset) - .is_some_and(|name| name.eq_ignore_ascii_case("Tempest\\Router\\Prefix")) - { + let Some(name) = ctx.resolved_name_at(decorator.name.span().start.offset) else { + continue; + }; + if !name.eq_ignore_ascii_case("Tempest\\Router\\Prefix") { + decorators.push(RouteDecorator::Custom(crate::atom::atom(name))); continue; } let Some(value) = decorator.argument_list.as_ref().and_then(|args| { @@ -98,6 +100,7 @@ pub(super) fn record_route_attributes(class: &Class<'_>, ctx: &mut ExtractionCtx else { continue 'attributes; }; + decorators.push(RouteDecorator::Prefix(crate::atom::atom(prefix))); route_path = format!( "{}/{}", prefix.trim_end_matches('/'), @@ -112,12 +115,15 @@ pub(super) fn record_route_attributes(class: &Class<'_>, ctx: &mut ExtractionCtx if !route_path.starts_with('/') { route_path.insert(0, '/'); } + let original_path = (path != route_path).then(|| path.into()); ctx.tempest_routes.push(RouteAttribute { attribute: crate::atom::atom(attribute_name), class: crate::atom::atom(class_name), method: crate::atom::atom_bytes(method.name.value), method_offset: method.name.span.start.offset, path: route_path, + original_path, + decorators: decorators.into_boxed_slice(), start: start as u32, placeholders: route_placeholders, parameters, diff --git a/src/tempest/mod.rs b/src/tempest/mod.rs index 76efe6b51..9de35dc9f 100644 --- a/src/tempest/mod.rs +++ b/src/tempest/mod.rs @@ -9,6 +9,7 @@ pub(crate) mod inference; mod model_properties; mod paths; pub(crate) mod preprocessor; +mod route_decorators; pub(crate) mod routes; mod syntax; mod view_data; diff --git a/src/tempest/route_decorators.rs b/src/tempest/route_decorators.rs new file mode 100644 index 000000000..273af76fe --- /dev/null +++ b/src/tempest/route_decorators.rs @@ -0,0 +1,221 @@ +//! Literal URI transformations in a project's RouteDecorator implementations. + +use std::borrow::Cow; +use std::ops::Deref; + +use mago_span::HasSpan; +use mago_syntax::{cst::*, walker::Walker}; + +use crate::Backend; +use crate::atom::{Atom, AtomMap, atom, bytes_to_str}; +use crate::class_lookup::{Fqn, is_subtype_of}; +use crate::types::FileContext; + +use super::routes::{RouteAttribute, RouteDecorator}; + +/// The indexed declaration with the URI its decorators produce for this request. +pub(super) struct ResolvedRoute<'a> { + declaration: &'a RouteAttribute, + /// The final URI, borrowing the indexed literal when no changes are needed. + pub(super) path: Cow<'a, str>, +} + +impl Deref for ResolvedRoute<'_> { + type Target = RouteAttribute; + + fn deref(&self) -> &Self::Target { + self.declaration + } +} + +#[derive(Clone, Copy)] +struct Prefix { + value: Atom, + /// Prefix::decorate joins path segments; a PHP concatenation preserves slashes. + join: bool, +} + +/// Resolve each attribute class once, without retaining stale bodies across edits. +#[derive(Default)] +pub(super) struct Decorators { + prefixes: AtomMap>, +} + +impl Decorators { + /// Apply the indexed attributes without copying the declaration's other data. + pub(super) fn resolve<'a>( + &mut self, + backend: &Backend, + declaration: &'a RouteAttribute, + ) -> ResolvedRoute<'a> { + if !declaration.decorators.iter().any(|decorator| { + matches!(decorator, RouteDecorator::Custom(class) + if self.prefix(backend, *class).is_some()) + }) { + return ResolvedRoute { + declaration, + path: Cow::Borrowed(&declaration.path), + }; + } + let mut path = Cow::Borrowed( + declaration + .original_path + .as_deref() + .unwrap_or(&declaration.path), + ); + for decorator in &declaration.decorators { + let prefix = match decorator { + RouteDecorator::Prefix(value) => Some(Prefix { + value: *value, + join: true, + }), + RouteDecorator::Custom(class) => self.prefix(backend, *class), + }; + if let Some(prefix) = prefix { + path = Cow::Owned(if prefix.join { + format!( + "{}/{}", + prefix.value.trim_end_matches('/'), + path.trim_start_matches('/') + ) + } else { + format!("{}{path}", prefix.value) + }); + } + } + if !path.starts_with('/') { + path.to_mut().insert(0, '/'); + } + ResolvedRoute { declaration, path } + } + + fn prefix(&mut self, backend: &Backend, class: Atom) -> Option { + *self + .prefixes + .entry(class) + .or_insert_with(|| read_prefix(backend, &class)) + } +} + +fn read_prefix(backend: &Backend, name: &str) -> Option { + let loader = backend.resolved_class_loader_with(&[]); + let class = loader(Fqn::new(name))?; + if !is_subtype_of(&class, "Tempest\\Router\\RouteDecorator", &loader) { + return None; + } + let declaring = crate::hover::find_declaring_class( + &class, + "decorate", + &crate::hover::MemberKindForOrigin::Method, + &loader, + ); + let method = declaring.get_method_ci("decorate")?; + let uri = backend.class_declaration_uri(&declaring.fqn())?; + let content = backend.get_file_content_arc(&uri)?; + let file = backend.file_context(&uri); + let _parse = crate::parser::with_parse_cache_arc(content.clone()); + crate::parser::with_parsed_program(&content, "tempest_route_decorator", |program, _| { + let mut found = (method.name_offset, None); + MethodPrefix { file: &file }.walk_program(program, &mut found); + found.1 + }) +} + +struct MethodPrefix<'a> { + file: &'a FileContext, +} + +impl<'ast, 'arena> Walker<'ast, 'arena, (u32, Option)> for MethodPrefix<'_> { + fn walk_in_method(&self, method: &'ast Method<'arena>, found: &mut (u32, Option)) { + if method.name.span.start.offset == found.0 { + found.1 = method_prefix(method, self.file); + } + } +} + +fn method_prefix(method: &Method<'_>, file: &FileContext) -> Option { + let route = method.parameter_list.parameters.first()?.variable.name; + let MethodBody::Concrete(body) = &method.body else { + return None; + }; + match body.statements.as_slice() { + [Statement::Return(ret)] => delegated_prefix(ret.value?, route, file), + [Statement::Expression(statement), Statement::Return(ret)] + if ret.value.is_some_and(|expr| is_variable(expr, route)) => + { + let Expression::Assignment(assignment) = statement.expression else { + return None; + }; + if !assignment.operator.is_assign() || !is_route_uri(assignment.lhs, route) { + return None; + } + let Expression::Binary(binary) = crate::parser::unwrap_parens(assignment.rhs) else { + return None; + }; + if !binary.operator.is_concatenation() || !is_route_uri(binary.rhs, route) { + return None; + } + Some(Prefix { + value: string_literal(binary.lhs)?, + join: false, + }) + } + _ => None, + } +} + +fn delegated_prefix(expr: &Expression<'_>, route: &[u8], file: &FileContext) -> Option { + let Expression::Call(Call::Method(call)) = crate::parser::unwrap_parens(expr) else { + return None; + }; + if !matches!(&call.method, ClassLikeMemberSelector::Identifier(name) if name.value.eq_ignore_ascii_case(b"decorate")) + || !is_variable(single_argument(&call.argument_list, b"route")?, route) + { + return None; + } + let Expression::Instantiation(new) = crate::parser::unwrap_parens(call.object) else { + return None; + }; + let Expression::Identifier(name) = new.class else { + return None; + }; + if !file + .resolve_name_at(bytes_to_str(name.value()), name.span().start.offset) + .trim_start_matches('\\') + .eq_ignore_ascii_case("Tempest\\Router\\Prefix") + { + return None; + } + Some(Prefix { + value: string_literal(single_argument(new.argument_list.as_ref()?, b"prefix")?)?, + join: true, + }) +} + +fn single_argument<'a>(args: &'a ArgumentList<'a>, name: &[u8]) -> Option<&'a Expression<'a>> { + match args.arguments.as_slice() { + [Argument::Positional(arg)] if arg.ellipsis.is_none() => Some(arg.value), + [Argument::Named(arg)] if arg.name.value == name => Some(arg.value), + _ => None, + } +} + +fn string_literal(expr: &Expression<'_>) -> Option { + let Expression::Literal(Literal::String(literal)) = crate::parser::unwrap_parens(expr) else { + return None; + }; + Some(atom(crate::atom::literal_bytes_to_str(literal.value?)?)) +} + +fn is_variable(expr: &Expression<'_>, name: &[u8]) -> bool { + matches!(crate::parser::unwrap_parens(expr), Expression::Variable(Variable::Direct(var)) if var.name == name) +} + +fn is_route_uri(expr: &Expression<'_>, route: &[u8]) -> bool { + matches!(crate::parser::unwrap_parens(expr), Expression::Access(Access::Property(access)) + if is_variable(access.object, route) + && matches!(&access.property, ClassLikeMemberSelector::Identifier(name) if name.value == b"uri")) +} + +#[cfg(test)] +mod tests; diff --git a/src/tempest/route_decorators/tests.rs b/src/tempest/route_decorators/tests.rs new file mode 100644 index 000000000..783004810 --- /dev/null +++ b/src/tempest/route_decorators/tests.rs @@ -0,0 +1,159 @@ +use super::*; + +#[test] +fn literal_decorator_bodies_require_the_same_route_and_a_known_prefix() { + let file = FileContext::default(); + for (body, expected) in [ + ( + r"return new \Tempest\Router\Prefix('/admin')->decorate($route);", + Some(("/admin", true)), + ), + ( + r"return (new \Tempest\Router\Prefix(prefix: ''))->decorate(route: ($route));", + Some(("", true)), + ), + ( + r"$route->uri = ('/admin/' . $route->uri); return $route;", + Some(("/admin/", false)), + ), + (r"return;", None), + (r"return $route;", None), + ( + r"return new \Tempest\Router\Prefix('/admin')->decorate($other);", + None, + ), + ( + r"return new \Tempest\Router\Prefix('/admin')->other($route);", + None, + ), + (r"return $prefix->decorate($route);", None), + (r"return new $prefix('/admin')->decorate($route);", None), + (r"return new Prefix('/admin')->decorate($route);", None), + ( + r"return new \Tempest\Router\Prefix->decorate($route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix()->decorate($route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix(other: '/admin')->decorate($route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix(...$parts)->decorate($route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix('/admin', '/extra')->decorate($route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix('/admin')->decorate();", + None, + ), + ( + r"return new \Tempest\Router\Prefix('/admin')->decorate(other: $route);", + None, + ), + ( + r"return new \Tempest\Router\Prefix($prefix)->decorate($route);", + None, + ), + ( + "return new \\Tempest\\Router\\Prefix(\"\\xFF\")->decorate($route);", + None, + ), + ( + r"$other->uri = '/admin' . $route->uri; return $route;", + None, + ), + ( + r"$route->other = '/admin' . $route->uri; return $route;", + None, + ), + (r"$route->uri .= '/admin'; return $route;", None), + (r"$route->uri = '/admin'; return $route;", None), + (r"$route->uri = 1 + $route->uri; return $route;", None), + ( + r"$route->uri = '/admin' . $other->uri; return $route;", + None, + ), + (r"$route->uri = $prefix . $route->uri; return $route;", None), + ( + r"$route->uri = '/admin' . $route->uri; return $other;", + None, + ), + (r"$route->uri = '/admin' . $route->uri; return;", None), + (r"echo 'ignored'; return $route;", None), + (r"log(); return $route;", None), + ( + r"if ($condition) { $route->uri = '/admin' . $route->uri; } return $route;", + None, + ), + ( + r"$route->uri = '/admin' . $route->uri; $route->uri = '/other'; return $route;", + None, + ), + ] { + let content = format!("decorate($route); } }", + ">, + /// Method decorators followed by class decorators, in discovery order. + pub decorators: Box<[RouteDecorator]>, /// Start of the URI's literal contents in the source file. pub start: u32, /// Source ranges of the placeholder names, excluding braces and patterns. @@ -36,6 +40,15 @@ pub(crate) struct RouteAttribute { pub parameters: Box<[RouteParameter]>, } +/// Prefix values are indexed immediately; other attributes need their PHP type. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum RouteDecorator { + /// A built-in prefix's decoded literal argument. + Prefix(Atom), + /// An attribute whose resolved class may implement RouteDecorator. + Custom(Atom), +} + /// A controller parameter's exact declaration token. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct RouteParameter { @@ -117,7 +130,7 @@ fn literal(ty: &PhpType) -> Option> { } } -fn matches_action(route: &RouteAttribute, ty: &PhpType) -> bool { +fn matches_action(route: &super::route_decorators::ResolvedRoute<'_>, ty: &PhpType) -> bool { if let TypeKind::Union(types) = ty.kind() { return types.iter().any(|ty| matches_action(route, ty)); } @@ -141,7 +154,7 @@ fn matches_action(route: &RouteAttribute, ty: &PhpType) -> bool { }) } -fn names_route_uri(route: &RouteAttribute, ty: &PhpType) -> bool { +fn names_route_uri(route: &super::route_decorators::ResolvedRoute<'_>, ty: &PhpType) -> bool { match ty.kind() { TypeKind::Union(types) => types.iter().any(|ty| names_route_uri(route, ty)), _ => literal(ty).is_some_and(|value| value == route.path), @@ -218,6 +231,7 @@ impl Backend { self.tempest_route_parameter_scopes(&controller_map, &content, name, scope) }) .unwrap_or_default(); + let mut decorators = super::route_decorators::Decorators::default(); let action_routes: Vec<_> = controller_map .tempest_routes .iter() @@ -231,6 +245,7 @@ impl Backend { })) && self.is_tempest_route(candidate) }) + .map(|route| decorators.resolve(self, route)) .collect(); for (file_uri, map) in self.user_file_symbol_maps() { if map.tempest_routes.is_empty() && map.tempest_uri_calls.is_empty() { @@ -351,17 +366,22 @@ impl Backend { } } - fn visit_tempest_routes(&self, mut visit: impl FnMut(&str, &RouteAttribute, bool)) { + fn visit_tempest_routes( + &self, + mut visit: impl FnMut(&str, &super::route_decorators::ResolvedRoute<'_>, bool), + ) { self.ensure_workspace_index_ready_for_request(); let mut confirmed = crate::atom::AtomMap::default(); let mut actions = std::collections::HashSet::new(); + let mut decorators = super::route_decorators::Decorators::default(); self.visit_declaring_symbol_maps(|uri, map| { for route in &map.tempest_routes { if *confirmed .entry(route.attribute) .or_insert_with(|| self.is_tempest_route(route)) { - visit(uri, route, actions.insert((route.class, route.method))); + let resolved = decorators.resolve(self, route); + visit(uri, &resolved, actions.insert((route.class, route.method))); } } }); @@ -682,7 +702,7 @@ impl Backend { [Some(route.method.as_str()), None] } else { [ - Some(route.path.as_str()), + Some(route.path.as_ref()), route .method .eq_ignore_ascii_case("__invoke") @@ -756,7 +776,7 @@ fn action_source<'a>(call: CallSite<'_>, content: &'a str) -> Option<(u32, &'a s Some((start + (text.len() - value.len()) as u32, value)) } -fn route_has_parameter(route: &RouteAttribute, name: &str) -> bool { +fn route_has_parameter(route: &super::route_decorators::ResolvedRoute<'_>, name: &str) -> bool { placeholders(&route.path) .iter() .any(|&(start, end)| &route.path[start..end] == name) diff --git a/src/tempest/routes/tests.rs b/src/tempest/routes/tests.rs index 5f869a935..de75cd6c8 100644 --- a/src/tempest/routes/tests.rs +++ b/src/tempest/routes/tests.rs @@ -40,6 +40,28 @@ fn route_extraction_requires_semantic_class_names() { assert!(map.tempest_routes.is_empty()); } +#[test] +fn route_extraction_keeps_routes_when_a_decorator_has_no_semantic_name() { + let source = " Uri { + open_php( + backend, + &workspace_uri(backend, "framework/router.php"), + &format!("{FRAMEWORK}\ninterface RouteDecorator {{ public function decorate(Route $route): Route; }}"), + ) + .await; + let uri = workspace_uri(backend, "routes/Decorators.php"); + open_php(backend, &uri, source).await; + uri +} + +#[tokio::test] +async fn tempest_routes_custom_decorators_preserve_order_navigation_and_parameter_renames() { + let source = r"DeCoRaTe(route: $route); + } +} +class Tenant implements RouteDecorator { + public function decorate(Route $candidate): Route { + $candidate->uri = '/{tenant}' . ($candidate->uri); + return ($candidate); + } +} +", + ) + .await; + let edited = CONTROLLER + .replace("class Books {", "#[Admin]\nclass Books {") + .replace( + "#[Read('/books/{book}')]", + "#[Read('/books/{book}')]\n #[\\Tempest\\Router\\Prefix('/v1')]\n #[Tenant]", + ) + .replace("string $unused", "string $tenant"); + open_php(&backend, &controller, &edited).await; + let offered = items(&backend, &uri, source, "uri('").await; + assert!( + offered + .iter() + .any(|item| item.label == "/admin/{tenant}/v1/books/{book}"), + "{offered:?}" + ); + assert!(!offered.iter().any(|item| item.label == "/books/{book}")); + let parameters = items(&backend, &uri, source, "{book}', ").await; + for name in ["book:", "tenant:"] { + assert!( + parameters.iter().any(|item| item.label == name), + "{parameters:?}" + ); + } + for (needle, target) in [ + ("/admin/{tenant}/v1/books/{book}', book:", "show(string"), + ("book: 'php'", "$book, string"), + ("tenant: 'acme'", "$tenant ="), + ] { + let locations = definitions(&backend, &uri, source, needle).await; + assert_eq!(locations.len(), 1, "{needle}: {locations:?}"); + assert_eq!(locations[0].uri, controller); + assert_eq!(locations[0].range.start, position_of(&edited, target)); + } + let at = position_of(&edited, "$book, string"); + let changes = rename(&backend, &controller, at.line, at.character + 1, "volume") + .await + .unwrap() + .changes + .unwrap(); + let edits = &changes[&uri]; + assert_eq!(edits.len(), 3, "{changes:?}"); + assert!(edits.iter().all(|edit| edit.new_text == "volume")); +} + +#[tokio::test] +async fn tempest_routes_custom_decorators_follow_unsaved_body_and_interface_edits() { + let source = r"decorate($route); + } +}"; + let decorator_uri = install_decorators(&backend, decorator).await; + open_php( + &backend, + &controller, + &CONTROLLER.replace("class Books {", "#[Admin]\nclass Books {"), + ) + .await; + for (edited, expected) in [ + (decorator.to_owned(), "/admin/books/{book}"), + ( + decorator.replace("'/admin'", "'/staff'"), + "/staff/books/{book}", + ), + ( + decorator.replace("implements \\Tempest\\Router\\RouteDecorator", ""), + "/books/{book}", + ), + (decorator.replace("'/admin'", "$unknown"), "/books/{book}"), + ] { + open_php(&backend, &decorator_uri, &edited).await; + let offered = items(&backend, &uri, source, "uri('").await; + assert!( + offered.iter().any(|item| item.label == expected), + "{edited}: {offered:?}" + ); + if expected != "/admin/books/{book}" { + assert!( + !offered + .iter() + .any(|item| item.label == "/admin/books/{book}") + ); + } + } +} + +#[tokio::test] +async fn tempest_routes_custom_decorators_use_declaring_namespace_and_member_precedence() { + let source = r"uri = '/global' . $route->uri; + return $route; + } + } +} +namespace Routes { + use Tempest\Router\Prefix as BuiltinPrefix; + use Tempest\Router\Route; + use Tempest\Router\RouteDecorator; + trait Decoration { + public function decorate(Route $route): Route { + return new BuiltinPrefix('/trait')->decorate($route); + } + } + class Base implements RouteDecorator { + public function decorate(Route $route): Route { + return new BuiltinPrefix('/parent')->decorate($route); + } + } + class Admin extends Base {} + class FromTrait extends Base { use Decoration; } + class Own extends Base { + use Decoration; + public function decorate(Route $route): Route { + $route->uri = '/own' . $route->uri; + return $route; + } + } + class Prefix {} + class Local implements RouteDecorator { + public function decorate(Route $route): Route { + return new Prefix('/wrong')->decorate($route); + } + } +} +", + ) + .await; + for (attribute, expected) in [ + ("Admin", "/parent/books/{book}"), + ("FromTrait", "/trait/books/{book}"), + ("Own", "/own/books/{book}"), + ("Local", "/books/{book}"), + ("\\Admin", "/global/books/{book}"), + ] { + open_php( + &backend, + &controller, + &CONTROLLER.replace("class Books {", &format!("#[{attribute}]\nclass Books {{")), + ) + .await; + let offered = items(&backend, &uri, source, "uri('").await; + assert!( + offered.iter().any(|item| item.label == expected), + "{attribute}: {offered:?}" + ); + assert!(!offered.iter().any(|item| item.label.starts_with("/wrong"))); + } +} + +#[tokio::test] +async fn tempest_routes_custom_decorators_preserve_literal_slashes_and_relative_uris() { + let source = r"decorate($route);", + "/books/{book}", + "/admin/books/{book}", + ), + ( + r"$route->uri = '/admin/' . $route->uri; return $route;", + "/books/{book}", + "/admin//books/{book}", + ), + ( + r"$route->uri = '/admin' . $route->uri; return $route;", + "books/{book}", + "/adminbooks/{book}", + ), + ( + r"$route->uri = '' . $route->uri; return $route;", + "books/{book}", + "/books/{book}", + ), + ( + r"return new \Tempest\Router\Prefix('')->decorate($route);", + "", + "/", + ), + ] { + open_php(&backend, &decorator_uri, &template.replace("BODY", body)).await; + let edited = CONTROLLER + .replace("class Books {", "#[Admin]\nclass Books {") + .replace("/books/{book}", path); + open_php(&backend, &controller, &edited).await; + let offered = items(&backend, &uri, source, "uri('").await; + assert!( + offered.iter().any(|item| item.label == expected), + "{body}, {path}: {offered:?}" + ); + } +}