diff --git a/nitrostack/cli/validators.py b/nitrostack/cli/validators.py index 2f76e48..2154299 100644 --- a/nitrostack/cli/validators.py +++ b/nitrostack/cli/validators.py @@ -11,6 +11,7 @@ from dataclasses import dataclass from typing import Any, Iterable, List, Optional, Sequence, Set, Tuple +from nitrostack.cli.widget_routes import collect_widget_routes from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.version import InvalidVersion, Version @@ -374,16 +375,12 @@ def validate_module_references(root: str) -> List[ValidationIssue]: return issues -_WIDGET_DECORATOR_RE = re.compile(r"""@widget\(\s*["']([^"']+)["']""") -_WIDGET_OPTIONS_RE = re.compile(r"""WidgetOptions\(\s*route\s*=\s*["']([^"']+)["']""") _WIDGET_ROUTE_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]*$") - def _collect_widget_routes(root: str) -> List[Tuple[str, str]]: - found: List[Tuple[str, str]] = [] - seen: Set[str] = set() root_abs = os.path.abspath(root) + sources: List[Tuple[str, str]] = [] for path in sorted(_iter_python_files(root)): rel_path = os.path.relpath(path, root_abs) @@ -394,19 +391,11 @@ def _collect_widget_routes(root: str) -> List[Tuple[str, str]]: continue try: - text = _read(path) + sources.append((rel_path, _read(path))) except OSError: continue - for route in ( - _WIDGET_DECORATOR_RE.findall(text) - + _WIDGET_OPTIONS_RE.findall(text) - ): - if route not in seen: - seen.add(route) - found.append((route, rel_path)) - - return found + return collect_widget_routes(sources) diff --git a/nitrostack/cli/widget_routes.py b/nitrostack/cli/widget_routes.py new file mode 100644 index 0000000..eb29626 --- /dev/null +++ b/nitrostack/cli/widget_routes.py @@ -0,0 +1,272 @@ +"""Static discovery of ``@widget`` routes for ``nitrostack-py validate``. + +Nothing is imported or run. Each project file is parsed and routes are found in three +ways: a string literal (``@widget("card")``), a ``WidgetOptions`` call with a literal +route, and a call to a *factory*, meaning a function whose parameter reaches +``WidgetOptions(route=...)`` (``@widget(my_widget("pizza-map"))``). + +A factory call only counts when the name resolves, through the calling module's own +definitions and imports, to a real factory, and the string sits in the argument that +feeds ``route``. So ``panel("x")`` is ignored when ``panel`` never passes that argument +to ``route``, or when it is an unrelated ``panel`` from another module. +""" + +from __future__ import annotations + +import ast +import os +import re +from dataclasses import dataclass, field +from typing import Dict, Iterable, List, Optional, Tuple + +# Only used for a file that does not parse, so a broken file still gets its literal routes. +_LEGACY_DECORATOR_RE = re.compile(r"""@widget\(\s*["']([^"']+)["']""") +_LEGACY_OPTIONS_RE = re.compile(r"""WidgetOptions\(\s*route\s*=\s*["']([^"']+)["']""") + +_WIDGET_OPTIONS = object() # the SDK's WidgetOptions class +_NITROSTACK = object() # the nitrostack package or one of its submodules +_PLAIN = object() # a project function or class that is not a factory + + +@dataclass(frozen=True) +class _Factory: + param: str # the parameter that feeds WidgetOptions.route + index: int # its position among the positional parameters, -1 if keyword-only + + +@dataclass +class _Namespace: + """A project module or a class, with the names it defines.""" + + name: str + defs: Dict[str, object] = field(default_factory=dict) + + +@dataclass +class _Module: + name: str + package: Tuple[str, ...] + tree: Optional[ast.Module] + text: str + ns: _Namespace + imports: Dict[str, Tuple[str, str]] = field(default_factory=dict) # local -> (module, attr) + funcs: List[Tuple[ast.AST, str, _Namespace]] = field(default_factory=list) # node, kind, owner + + +def _dotted(rel_path: str) -> Tuple[str, Tuple[str, ...]]: + parts = os.path.normpath(rel_path).split(os.sep) + parts[-1] = parts[-1][:-3] if parts[-1].endswith(".py") else parts[-1] + is_package = parts[-1] == "__init__" + if is_package: + parts = parts[:-1] + return ".".join(parts), tuple(parts if is_package else parts[:-1]) + + +def _is_nitrostack(module: str) -> bool: + return module == "nitrostack" or module.startswith("nitrostack.") + + +class _Project: + def __init__(self, sources: Iterable[Tuple[str, str]]): + self.modules: Dict[str, _Module] = {} + self.order: List[Tuple[str, _Module]] = [] + for rel_path, text in sources: + name, package = _dotted(rel_path) + try: + tree: Optional[ast.Module] = ast.parse(text) + except (SyntaxError, ValueError): + tree = None + module = _Module(name, package, tree, text, _Namespace(name)) + self.order.append((rel_path, module)) + self.modules[name] = module + if tree is not None: + self._index(module) + self._find_factories() + + # -- indexing --------------------------------------------------------------- + + def _index(self, module: _Module) -> None: + assert module.tree is not None + for node in ast.walk(module.tree): + if isinstance(node, ast.Import): + for alias in node.names: + if alias.asname: + module.imports[alias.asname] = (alias.name, "") + else: + top = alias.name.split(".")[0] + module.imports[top] = (top, "") + elif isinstance(node, ast.ImportFrom): + source = self._absolute(module, node) + if source is None: + continue + for alias in node.names: + if alias.name != "*": + module.imports[alias.asname or alias.name] = (source, alias.name) + for node in module.tree.body: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + module.ns.defs[node.name] = _PLAIN + module.funcs.append((node, "func", module.ns)) + elif isinstance(node, ast.ClassDef): + cls = _Namespace(f"{module.name}.{node.name}") + module.ns.defs[node.name] = cls + for item in node.body: + kind = self._method_kind(item) + if kind: + cls.defs[item.name] = _PLAIN + module.funcs.append((item, kind, cls)) + + @staticmethod + def _method_kind(item: ast.AST) -> Optional[str]: + if not isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)): + return None + names = {d.id for d in item.decorator_list if isinstance(d, ast.Name)} + if "staticmethod" in names: + return "static" + return "class" if "classmethod" in names else None # instance methods need an object + + @staticmethod + def _absolute(module: _Module, node: ast.ImportFrom) -> Optional[str]: + if not node.level: + return node.module + keep = len(module.package) - (node.level - 1) + if keep < 0: + return None + base = list(module.package[:keep]) + ([node.module] if node.module else []) + return ".".join(base) + + # -- name resolution -------------------------------------------------------- + + def _find(self, dotted: str) -> Optional[_Module]: + if dotted in self.modules: + return self.modules[dotted] + matches = [m for key, m in self.modules.items() if key.endswith("." + dotted)] + return matches[0] if len(matches) == 1 else None # e.g. a src/ layout + + def _name(self, module: _Module, name: str) -> object: + if name in module.ns.defs: + return module.ns.defs[name] + imported = module.imports.get(name) + if imported: + source, attr = imported + if _is_nitrostack(source): + return _WIDGET_OPTIONS if attr == "WidgetOptions" else _NITROSTACK + if not attr: + found = self._find(source) + return found.ns if found else None + found = self._find(source) + if found and attr in found.ns.defs: + return found.ns.defs[attr] + sub = self._find(f"{source}.{attr}") # ``from pkg import module`` + return sub.ns if sub else None + return _WIDGET_OPTIONS if name == "WidgetOptions" else None + + def resolve(self, expr: ast.AST, module: _Module) -> object: + if isinstance(expr, ast.Name): + return self._name(module, expr.id) + if isinstance(expr, ast.Attribute): + base = self.resolve(expr.value, module) + if base is _NITROSTACK: + return _WIDGET_OPTIONS if expr.attr == "WidgetOptions" else _NITROSTACK + if isinstance(base, _Namespace): + if expr.attr in base.defs: + return base.defs[expr.attr] + sub = self._find(f"{base.name}.{expr.attr}") + return sub.ns if sub else None + return None + + @staticmethod + def route_arg(call: ast.Call, target: object) -> Optional[ast.AST]: + """The expression passed where ``target`` expects the route, if it is passed at all.""" + if target is _WIDGET_OPTIONS: + name, index = "route", 0 + elif isinstance(target, _Factory): + name, index = target.param, target.index + else: + return None + if 0 <= index < len(call.args): + if any(isinstance(arg, ast.Starred) for arg in call.args[: index + 1]): + return None + return call.args[index] + for keyword in call.keywords: + if keyword.arg == name: + return keyword.value + return None + + # -- factories -------------------------------------------------------------- + + def _factory_of(self, module: _Module, fn: ast.AST, kind: str) -> Optional[_Factory]: + positional = [a.arg for a in fn.args.posonlyargs + fn.args.args] # type: ignore[attr-defined] + if kind == "class": + positional = positional[1:] + params = positional + [a.arg for a in fn.args.kwonlyargs] # type: ignore[attr-defined] + reassigned = { + n.id for n in ast.walk(fn) if isinstance(n, ast.Name) and isinstance(n.ctx, ast.Store) + } + for call in (n for n in ast.walk(fn) if isinstance(n, ast.Call)): + expr = self.route_arg(call, self.resolve(call.func, module)) + if isinstance(expr, ast.Name) and expr.id in params and expr.id not in reassigned: + return _Factory(expr.id, positional.index(expr.id) if expr.id in positional else -1) + return None + + def _find_factories(self) -> None: + # Repeat so a function that forwards its parameter to another factory is found too. + for _ in range(sum(len(m.funcs) for _, m in self.order) + 1): + changed = False + for _, module in self.order: + for fn, kind, owner in module.funcs: + if owner.defs.get(fn.name) is not _PLAIN: # type: ignore[attr-defined] + continue + factory = self._factory_of(module, fn, kind) + if factory: + owner.defs[fn.name] = factory # type: ignore[attr-defined] + changed = True + if not changed: + break + + # -- routes ----------------------------------------------------------------- + + def _literal(self, expr: Optional[ast.AST]) -> Optional[str]: + if isinstance(expr, ast.Constant) and isinstance(expr.value, str): + return expr.value + return None + + def routes_in(self, module: _Module) -> List[str]: + if module.tree is None: + return _LEGACY_DECORATOR_RE.findall(module.text) + _LEGACY_OPTIONS_RE.findall(module.text) + found: List[Tuple[int, int, str]] = [] + for node in ast.walk(module.tree): + for decorator in getattr(node, "decorator_list", []): + func = getattr(decorator, "func", None) + is_widget = (isinstance(func, ast.Name) and func.id == "widget") or ( + isinstance(func, ast.Attribute) and func.attr == "widget" + ) + if isinstance(decorator, ast.Call) and is_widget and decorator.args: + route = self._decorator_route(decorator.args[0], module) + if route: + found.append((decorator.lineno, decorator.col_offset, route)) + if isinstance(node, ast.Call) and self.resolve(node.func, module) is _WIDGET_OPTIONS: + route = self._literal(self.route_arg(node, _WIDGET_OPTIONS)) + if route: + found.append((node.lineno, node.col_offset, route)) + return [route for _, _, route in sorted(found)] + + def _decorator_route(self, expr: ast.AST, module: _Module) -> Optional[str]: + literal = self._literal(expr) + if literal is not None: + return literal + if isinstance(expr, ast.Call): + return self._literal(self.route_arg(expr, self.resolve(expr.func, module))) + return None + + +def collect_widget_routes(sources: Iterable[Tuple[str, str]]) -> List[Tuple[str, str]]: + """``(route, rel_path)`` for every route found in ``(rel_path, source_text)`` pairs.""" + project = _Project(sources) + found: List[Tuple[str, str]] = [] + seen = set() + for rel_path, module in project.order: + for route in project.routes_in(module): + if route not in seen: + seen.add(route) + found.append((route, rel_path)) + return found diff --git a/tests/test_validate_widgets.py b/tests/test_validate_widgets.py index 1595e60..ba5dfae 100644 --- a/tests/test_validate_widgets.py +++ b/tests/test_validate_widgets.py @@ -1,7 +1,12 @@ +import os +import shutil +import subprocess import sys from pathlib import Path -from nitrostack.cli.validators import validate_project, validate_widgets +import pytest + +from nitrostack.cli.validators import _collect_widget_routes, validate_project, validate_widgets TOOL = ( "from nitrostack import tool, widget\n\n" @@ -82,4 +87,224 @@ def test_validate_project_reports_widget_warnings_only(tmp_path: Path, monkeypat _project(tmp_path) issues = [i for i in validate_project(str(tmp_path)) if "widget route" in i.message] assert len(issues) == 1 - assert issues[0].severity == "warning" \ No newline at end of file + assert issues[0].severity == "warning" + +HELPER = ( + "from nitrostack import WidgetOptions, widget\n\n" + "def pizzaz_widget(route: str) -> WidgetOptions:\n" + " return WidgetOptions(route=route, prefers_border=True)\n\n" + "class helpers:\n" + " @staticmethod\n" + " def make(route):\n" + " return WidgetOptions(route=route)\n\n" + "class T:\n" + " @widget(pizzaz_widget('pizza-map'))\n" + " def a(self): ...\n\n" + " @widget(helpers.make(\"pizza-list\"))\n" + " def b(self): ...\n\n" + " @widget(pizzaz_widget(route='pizza-shop', extra=1))\n" + " def c(self): ...\n" +) + + +def test_helper_wrapped_routes_are_collected(tmp_path: Path): + issues = validate_widgets(str(_project(tmp_path, HELPER))) + assert sorted(i.message.split("'")[1] for i in issues) == ["pizza-list", "pizza-map", "pizza-shop"] + assert all("file is missing" in i.message for i in issues) + + +def test_helper_wrapped_route_with_html_passes(tmp_path: Path): + _project(tmp_path, HELPER) + for route in ("pizza-map", "pizza-list", "pizza-shop"): + _write_html(tmp_path, route, "") + assert validate_widgets(str(tmp_path)) == [] + + +def test_route_is_read_from_the_argument_that_feeds_it(tmp_path: Path): + src = ( + "from nitrostack import WidgetOptions, widget\n" + "def helper(flag, route): return WidgetOptions(route=route)\n" + "@widget(helper(True, 'second'))\ndef a(): ...\n" + "@widget(helper('first', 'third'))\ndef b(): ...\n" + "@widget(helper(flag=False, route='by-keyword'))\ndef c(): ...\n" + ) + assert sorted(_routes(tmp_path, {"tools.py": src})) == ["by-keyword", "second", "third"] + + +def test_unrelated_helper_call_is_not_taken_as_a_route(tmp_path: Path): + # Not defined in the project and never builds a WidgetOptions: no false warning. + src = "@widget(something('x'))\ndef f(): ...\n" + assert validate_widgets(str(_project(tmp_path, src))) == [] + src = "def something(name): return {'n': name}\n@widget(something('x'))\ndef f(): ...\n" + assert validate_widgets(str(_project(tmp_path, src))) == [] + + +def test_helper_defined_in_another_file_is_found(tmp_path: Path): + (tmp_path / "helpers.py").write_text( + "from nitrostack import WidgetOptions\ndef panel(route): return WidgetOptions(route=route)\n", + encoding="utf-8", + ) + src = "from helpers import panel\n@widget(panel('chart'))\ndef f(): ...\n" + issues = validate_widgets(str(_project(tmp_path, src))) + assert len(issues) == 1 and "'chart'" in issues[0].message + + +def test_widget_options_positional_route_is_collected(tmp_path: Path): + src = "from nitrostack import WidgetOptions, widget\n@widget(WidgetOptions('chart'))\ndef f(): ...\n" + issues = validate_widgets(str(_project(tmp_path, src))) + assert len(issues) == 1 and "'chart'" in issues[0].message + + +def test_invalid_helper_route_is_reported(tmp_path: Path): + src = "def helper(r): return WidgetOptions(route=r)\n@widget(helper('../evil'))\ndef f(): ...\n" + issues = validate_widgets(str(_project(tmp_path, src))) + assert len(issues) == 1 + assert "not a single alphanumeric path segment" in issues[0].message + + +TEMPLATES = Path(__file__).resolve().parents[1] / "nitrostack" / "templates" + + +@pytest.mark.parametrize("template, expected", [ + ("pizzaz", {"pizza-map", "pizza-list", "pizza-shop"}), + ("starter", {"calculator-result"}), + ("flight-booking", { + "flight-search-results", "flight-details", "airport-search", + "order-summary", "seat-selection", "order-cancellation", + }), +]) +def test_shipped_templates_report_every_widget_route(template, expected): + routes = {route for route, _ in _collect_widget_routes(str(TEMPLATES / template))} + assert routes == expected + + +def test_pizzaz_template_warns_when_a_widget_html_is_removed(tmp_path: Path): + project = tmp_path / "pizzaz" + shutil.copytree(TEMPLATES / "pizzaz", project) + assert validate_widgets(str(project)) == [] + (project / "widgets" / "out" / "pizza-list.html").unlink() + issues = validate_widgets(str(project)) + assert len(issues) == 1 + assert "'pizza-list'" in issues[0].message and "file is missing" in issues[0].message + + +def test_cli_validate_checks_pizzaz_helper_wrapped_widgets(tmp_path: Path): + project = tmp_path / "pizzaz" + shutil.copytree(TEMPLATES / "pizzaz", project) + + def validate(): + return subprocess.run( + [sys.executable, "-m", "nitrostack.cli.main", "validate"], + cwd=project, env={**os.environ, "PYTHONPATH": str(TEMPLATES.parents[1])}, + capture_output=True, text=True, encoding="utf-8", timeout=60, + ) + + clean = validate() + assert clean.returncode == 0, clean.stdout + clean.stderr + assert "widget route" not in clean.stdout + + (project / "widgets" / "out" / "pizza-list.html").unlink() + broken = validate().stdout + assert "widget route 'pizza-list'" in broken and "file is missing" in broken + assert "pizza-map" not in broken and "pizza-shop" not in broken + + +def _routes(tmp_path: Path, files: dict) -> list: + for name, text in files.items(): + target = tmp_path / name + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text, encoding="utf-8") + return [route for route, _ in _collect_widget_routes(str(tmp_path))] + + +FACTORY = "from nitrostack import WidgetOptions\ndef panel(route): return WidgetOptions(route=route)\n" + + +def test_argument_that_does_not_feed_route_is_not_a_route(tmp_path: Path): + # panel("dashboard") builds the "card" widget; dashboard.html is never used. + src = ( + "from nitrostack import WidgetOptions, widget\n" + "def panel(name, description=None):\n" + " return WidgetOptions(route='card', description=description)\n" + "@widget(panel('dashboard'))\ndef f(): ...\n" + ) + assert _routes(tmp_path, {"tools.py": src}) == ["card"] + assert validate_widgets(str(tmp_path))[0].message.startswith("widget route 'card'") + assert len(validate_widgets(str(tmp_path))) == 1 + + +def test_reassigned_or_forwarded_to_something_else_is_not_a_factory(tmp_path: Path): + src = ( + "from nitrostack import WidgetOptions, widget\n" + "def fixed(route):\n route = 'always-this'\n return WidgetOptions(route=route)\n" + "def other(route): return WidgetOptions(route='card', description=route)\n" + "@widget(fixed('x'))\ndef a(): ...\n@widget(other('y'))\ndef b(): ...\n" + ) + assert _routes(tmp_path, {"tools.py": src}) == ["card"] + + +def test_same_helper_name_in_another_module_is_not_a_factory(tmp_path: Path): + files = { + "factory.py": FACTORY, + "unrelated.py": ( + "from nitrostack import widget\n" + "def panel(name): return {'name': name}\n" + "@widget(panel('not-a-route'))\ndef f(): ...\n" + ), + "undefined.py": "from nitrostack import widget\n@widget(panel('also-not'))\ndef g(): ...\n", + "other_lib.py": ( + "from some_library import panel\nfrom nitrostack import widget\n" + "@widget(panel('third-party'))\ndef h(): ...\n" + ), + } + assert _routes(tmp_path, files) == [] + + +def test_factory_is_resolved_through_the_calling_modules_imports(tmp_path: Path): + files = { + "factory.py": FACTORY, + "plain.py": "from factory import panel\nfrom nitrostack import widget\n@widget(panel('plain'))\ndef a(): ...\n", + "aliased.py": "from factory import panel as build\nfrom nitrostack import widget\n@widget(build('aliased'))\ndef b(): ...\n", + "module.py": "import factory as fx\nfrom nitrostack import widget\n@widget(fx.panel('via-module'))\ndef c(): ...\n", + "pkg/__init__.py": "", + "pkg/helpers.py": FACTORY, + "pkg/tools.py": "from .helpers import panel\nfrom nitrostack import widget\n@widget(panel('relative'))\ndef d(): ...\n", + "pkg/sub.py": "from pkg import helpers\nfrom nitrostack import widget\n@widget(helpers.panel('pkg-module'))\ndef e(): ...\n", + } + assert sorted(_routes(tmp_path, files)) == ["aliased", "pkg-module", "plain", "relative", "via-module"] + + +def test_a_local_definition_wins_over_an_import_of_the_same_name(tmp_path: Path): + files = { + "factory.py": FACTORY, + "tools.py": ( + "from factory import panel\nfrom nitrostack import widget\n" + "def panel(name): return {'name': name}\n" + "@widget(panel('shadowed'))\ndef f(): ...\n" + ), + } + assert _routes(tmp_path, files) == [] + + +def test_a_factory_that_forwards_to_another_factory_is_found(tmp_path: Path): + src = ( + "from nitrostack import WidgetOptions, widget\n" + "def base(route): return WidgetOptions(route=route)\n" + "def wrapped(r): return base(r)\n" + "@widget(wrapped('deep'))\ndef f(): ...\n" + ) + assert _routes(tmp_path, {"tools.py": src}) == ["deep"] + + +def test_unimported_widget_options_alias_and_attribute_forms(tmp_path: Path): + src = ( + "import nitrostack\nfrom nitrostack import WidgetOptions as WO, widget\n" + "@widget(WO(route='aliased-options'))\ndef a(): ...\n" + "@nitrostack.widget(nitrostack.WidgetOptions('attribute-form'))\ndef b(): ...\n" + ) + assert sorted(_routes(tmp_path, {"tools.py": src})) == ["aliased-options", "attribute-form"] + + +def test_a_file_that_does_not_parse_still_yields_its_literal_routes(tmp_path: Path): + src = "@widget('legacy')\ndef f(:\n pass\nWidgetOptions(route='legacy-options')\n" + assert sorted(_routes(tmp_path, {"broken.py": src})) == ["legacy", "legacy-options"]