fix: compile deterministic routes and diagnose ambiguous paths

docs/runtime.md

@@ -0,0 +1,37 @@
+# Runtime behavior
+
+## Routing
+
+Routes are flattened across modules and nested `location` blocks and compiled
+into a deterministic table for each HTTP method. Segments are compared from left
+to right: literal > named parameter > wildcard. A literal route therefore wins
+over a generic route regardless of file discovery or declaration order.
+
+Paths must match all segments. `*` matches exactly one nonempty segment, not a
+whole subtree. Trailing and repeated slashes are ignored. Query strings do not
+participate in route selection. Parameters are raw URI strings: their declared
+types are not checked or converted, and percent escapes are not decoded.
+
+Equivalent routes (including parameters with different names or type annotations)
+are rejected with both source locations. Overlapping routes such as
+`/fixed/(x: String)` and `/(y: String)/fixed` produce a warning; left-to-right
+precedence selects the former for `/fixed/fixed`. Equal-ranked patterns use
+lexical path, source filename, and line order as a stable fallback. Methods are
+independent; there is no implicit GET-to-HEAD fallback or automatic 405 response.
+Missing routes return 404.
+
+## Development reload
+
+Before every development request, WebX checks the `.wx`/`.webx` source tree by
+content, as well as responding to filesystem notifications. A completed valid
+save is visible on the next request without restarting or sleeping for a watcher.
+This includes new files, deletions, and editor atomic-save renames. Production
+mode does not reload source modules. Static files are read on each call in either
+mode; no browser auto-refresh is provided.
+
+Parsing, route validation, and changed global-block initialization must all
+succeed before the new module set and route table become active. A failed edit
+reports a diagnostic and retains the complete last good snapshot. HTML/route-only
+edits preserve module globals. Changing a module's `global` block resets that
+module's state; removal discards it. Development checks read the source tree on
+each request and are intended for correctness and convenience, not throughput.

src/analysis/routes.rs

@@ -1,137 +1,175 @@
-use colored::*;
-
-use std::collections::HashMap;
+use std::cmp::Ordering;
 
 use crate::{
     engine::runtime::WXRuntimeError,
-    file::webx::{WXInfoField, WXModule, WXRoute, WXScope, WXUrlPath, WXROOT_PATH},
-    reporting::{
-        error::{
-            exit_error, format_info_field, DateTimeSpecifier, ERROR_DUPLICATE_ROUTE,
-            ERROR_INVALID_ROUTE,
-        },
-        route::print_route,
+    file::webx::{WXModule, WXRoute, WXScope, WXUrlPath, WXUrlPathSegment, WXROOT_PATH},
+    reporting::error::{
+        exit_error, format_info_field, DateTimeSpecifier, ERROR_DUPLICATE_ROUTE,
+        ERROR_INVALID_ROUTE,
     },
 };
 
-pub type FlatRoutes = HashMap<(WXRoute, WXUrlPath), Vec<WXInfoField>>;
+// Keep every declaration, including duplicates in different location scopes.
+pub type FlatRoutes = Vec<(WXRoute, WXUrlPath)>;
 
-fn flatten_scopes(
-    module_name: String,
-    scope: &WXScope,
-    path_prefix: WXUrlPath,
-    routes: &mut FlatRoutes,
-) {
-    for route in scope.routes.iter() {
-        let flat_path = path_prefix.combine(&route.path);
-        let route_key = (route.clone(), flat_path);
-        if let std::collections::hash_map::Entry::Vacant(entry) = routes.entry(route_key.clone()) {
-            entry.insert(vec![route.info.clone()]);
-        } else {
-            routes.get_mut(&route_key).unwrap().push(route.info.clone());
-        }
+fn flatten_scopes(scope: &WXScope, prefix: &WXUrlPath, routes: &mut FlatRoutes) {
+    for route in &scope.routes {
+        routes.push((route.clone(), prefix.combine(&route.path)));
     }
-    for sub_scope in scope.scopes.iter() {
-        let sub_scope_path = path_prefix.combine(&sub_scope.path);
-        flatten_scopes(module_name.clone(), sub_scope, sub_scope_path, routes);
+    for child in &scope.scopes {
+        flatten_scopes(child, &prefix.combine(&child.path), routes);
     }
 }
 
 pub fn extract_flat_routes(modules: &[WXModule]) -> FlatRoutes {
-    let mut routes = HashMap::new();
-    for module in modules.iter() {
-        flatten_scopes(
-            module.path.module_name(),
-            &module.scope,
-            WXROOT_PATH,
-            &mut routes,
-        );
+    let mut routes = Vec::new();
+    for module in modules {
+        flatten_scopes(&module.scope, &WXROOT_PATH, &mut routes);
     }
+    routes.sort_by(|(a, ap), (b, bp)| {
+        a.method
+            .as_str()
+            .cmp(b.method.as_str())
+            .then_with(|| compare_specificity(ap, bp))
+            .then_with(|| ap.to_string().cmp(&bp.to_string()))
+            .then_with(|| a.info.path.inner.cmp(&b.info.path.inner))
+            .then_with(|| a.info.line.cmp(&b.info.line))
+    });
     routes
 }
 
-pub fn extract_duplicate_routes(routes: &FlatRoutes) -> Vec<String> {
-    routes
-        .iter()
-        .filter(|(_, modules)| modules.len() > 1)
-        .map(|((route, path), modules)| {
-            let locations = modules.iter().map(format_info_field).collect::<Vec<_>>();
-            format!(
-                "Route {} is defined in modules:\n    - {}",
-                print_route(&route.method, path),
-                locations.join("\n    - ")
-            )
-        })
-        .collect()
+fn rank(segment: &WXUrlPathSegment) -> u8 {
+    match segment {
+        WXUrlPathSegment::Literal(_) => 2,
+        WXUrlPathSegment::Parameter(_) => 1,
+        WXUrlPathSegment::Regex(_, _) => 0,
+    }
 }
 
-pub fn analyze_duplicate_routes(modules: &[WXModule]) -> Result<FlatRoutes, WXRuntimeError> {
-    let routes = extract_flat_routes(modules);
-    let duplicate_routes = extract_duplicate_routes(&routes);
-    if !duplicate_routes.is_empty() {
-        return Err(WXRuntimeError {
-            code: ERROR_DUPLICATE_ROUTE,
-            message: format!(
-                "Duplicate routes detected:\n  - {}",
-                duplicate_routes.join("\n  - ")
-            ),
-        });
-    }
-    Ok(routes)
+/// Most specific first: compare segments from left to right, then length.
+/// Matching requires equal lengths; the length comparison only orders the table.
+pub fn compare_specificity(a: &WXUrlPath, b: &WXUrlPath) -> Ordering {
+    b.0.iter().map(rank).cmp(a.0.iter().map(rank))
 }
 
-fn extract_invalid_routes(routes: &FlatRoutes) -> Vec<String> {
-    routes
-        .iter()
-        .filter(|((route, _), _)| match route.method {
-            hyper::Method::GET | hyper::Method::DELETE => route.body_format.is_some(),
-            hyper::Method::POST | hyper::Method::PUT => route.body_format.is_none(),
+fn same_shape(a: &WXUrlPath, b: &WXUrlPath) -> bool {
+    a.segments() == b.segments()
+        && a.0.iter().zip(&b.0).all(|(a, b)| match (a, b) {
+            (WXUrlPathSegment::Literal(a), WXUrlPathSegment::Literal(b)) => a == b,
+            (WXUrlPathSegment::Parameter(_), WXUrlPathSegment::Parameter(_)) => true,
+            (WXUrlPathSegment::Regex(_, a), WXUrlPathSegment::Regex(_, b)) => a == b,
             _ => false,
         })
-        .map(|((route, path), info)| {
-            format!(
-                "Route {} {} specify {}, but is not a POST or PUT endpoint. {}",
-                route.method.to_string().green(),
-                path.to_string().yellow(),
-                route.body_format.as_ref().unwrap().to_string().red(),
-                format_info_field(info.first().unwrap()),
-            )
+}
+
+fn may_overlap(a: &WXUrlPath, b: &WXUrlPath) -> bool {
+    a.segments() == b.segments()
+        && a.0.iter().zip(&b.0).all(|(a, b)| match (a, b) {
+            (WXUrlPathSegment::Literal(a), WXUrlPathSegment::Literal(b)) => a == b,
+            (WXUrlPathSegment::Literal(text), WXUrlPathSegment::Regex(_, pattern))
+            | (WXUrlPathSegment::Regex(_, pattern), WXUrlPathSegment::Literal(text)) => {
+                pattern == "*"
+                    || regex::Regex::new(&format!("^(?:{})$", pattern))
+                        .is_ok_and(|re| re.is_match(text))
+            }
+            _ => true,
         })
-        .collect()
 }
 
-/// Analyze the implementation of routes in a list of WebX modules.
-/// If an invalid route is detected, an error is reported and the program exits.
-/// Invalid routes include:
-/// - bad combinations of route methods and request body format types (e.g. GET + body)
-pub fn analyze_invalid_routes(modules: &[WXModule]) -> Result<(), WXRuntimeError> {
+pub fn extract_duplicate_routes(routes: &FlatRoutes) -> Vec<String> {
+    let mut diagnostics = Vec::new();
+    for (i, (a, ap)) in routes.iter().enumerate() {
+        for (b, bp) in &routes[i + 1..] {
+            if a.method == b.method && same_shape(ap, bp) {
+                diagnostics.push(format!("Duplicate/ambiguous route {} {} at {} conflicts with {} at {} (parameter names and types do not distinguish routes)",
+                    a.method, ap, format_info_field(&a.info), bp, format_info_field(&b.info)));
+            }
+        }
+    }
+    diagnostics
+}
+
+/// Warn about overlaps that are not a simple specific-route/fallback pair.
+pub fn ambiguous_route_warnings(routes: &FlatRoutes) -> Vec<String> {
+    let mut diagnostics = Vec::new();
+    for (i, (a, ap)) in routes.iter().enumerate() {
+        for (b, bp) in &routes[i + 1..] {
+            let a_more = ap.0.iter().zip(&bp.0).any(|(a, b)| rank(a) > rank(b));
+            let b_more = ap.0.iter().zip(&bp.0).any(|(a, b)| rank(b) > rank(a));
+            if a.method == b.method
+                && !same_shape(ap, bp)
+                && may_overlap(ap, bp)
+                && ((a_more && b_more) || compare_specificity(ap, bp) == Ordering::Equal)
+            {
+                diagnostics.push(format!("Ambiguous routes {} {} at {} and {} at {}; precedence is left-to-right literal > parameter > wildcard, then lexical path/source order",
+                    a.method, ap, format_info_field(&a.info), bp, format_info_field(&b.info)));
+            }
+        }
+    }
+    diagnostics
+}
+
+pub fn analyze_duplicate_routes(modules: &[WXModule]) -> Result<FlatRoutes, WXRuntimeError> {
     let routes = extract_flat_routes(modules);
-    let invalid_routes = extract_invalid_routes(&routes);
-    if !invalid_routes.is_empty() {
+    let duplicates = extract_duplicate_routes(&routes);
+    if !duplicates.is_empty() {
         return Err(WXRuntimeError {
-            code: ERROR_INVALID_ROUTE,
-            message: format!(
-                "Invalid routes detected:\n  - {}",
-                invalid_routes.join("\n  - ")
-            ),
+            code: ERROR_DUPLICATE_ROUTE,
+            message: duplicates.join("\n"),
         });
     }
-    Ok(())
+    Ok(routes)
 }
 
-fn exit_on_err<T>(result: Result<T, WXRuntimeError>) {
-    if let Err(err) = result {
-        exit_error(err.message, err.code, DateTimeSpecifier::None);
+pub fn analyze_invalid_routes(modules: &[WXModule]) -> Result<(), WXRuntimeError> {
+    let mut errors = Vec::new();
+    for (route, path) in extract_flat_routes(modules) {
+        // A body declaration is optional for write methods. Never unwrap a missing declaration.
+        if matches!(
+            route.method,
+            hyper::Method::GET | hyper::Method::HEAD | hyper::Method::DELETE
+        ) && route.body_format.is_some()
+        {
+            errors.push(format!(
+                "{} {} cannot declare a request body ({})",
+                route.method,
+                path,
+                format_info_field(&route.info)
+            ));
+        }
+        for segment in &path.0 {
+            if let WXUrlPathSegment::Regex(_, pattern) = segment {
+                if pattern != "*" && regex::Regex::new(&format!("^(?:{})$", pattern)).is_err() {
+                    errors.push(format!(
+                        "Invalid route regex {:?} at {}",
+                        pattern,
+                        format_info_field(&route.info)
+                    ));
+                }
+            }
+        }
+    }
+    if errors.is_empty() {
+        Ok(())
+    } else {
+        Err(WXRuntimeError {
+            code: ERROR_INVALID_ROUTE,
+            message: errors.join("\n"),
+        })
     }
 }
 
 pub fn analyze_module_routes(modules: &[WXModule]) {
-    exit_on_err(analyze_duplicate_routes(modules));
-    exit_on_err(analyze_invalid_routes(modules));
+    if let Err(err) = verify_model_routes(modules) {
+        exit_error(err.message, err.code, DateTimeSpecifier::None);
+    }
 }
 
 pub fn verify_model_routes(modules: &[WXModule]) -> Result<FlatRoutes, WXRuntimeError> {
     let routes = analyze_duplicate_routes(modules)?;
     analyze_invalid_routes(modules)?;
+    for message in ambiguous_route_warnings(&routes) {
+        eprintln!("Route warning: {}", message);
+    }
     Ok(routes)
 }