diff --git a/CHANGES.md b/CHANGES.md index e8695d1..2868ee6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Release Notes +## 0.2.2 + +Unrecognized lift manifest configuration data now generates an informative error instead of +silently skipping the unrecognized configuration. + ## 0.2.1 Fix command descriptions not being rendered in the JSON lift manifest of built scies. diff --git a/lift.toml b/lift.toml index 54ed04e..b31dbe4 100644 --- a/lift.toml +++ b/lift.toml @@ -40,4 +40,4 @@ remove_re = [ ] [lift.commands.env.replace] SHIV_ROOT = "{scie.bindings}/shiv_root" -SCIENCE_DOC_LOCAL = "{docsite}/index.html" +SCIENCE_DOC_LOCAL = "{docsite}" diff --git a/science/__init__.py b/science/__init__.py index c992efe..eb2b358 100644 --- a/science/__init__.py +++ b/science/__init__.py @@ -3,6 +3,6 @@ from packaging.version import Version -__version__ = "0.2.1" +__version__ = "0.2.2" VERSION = Version(__version__) diff --git a/science/config.py b/science/config.py index 05d5b47..7d226b7 100644 --- a/science/config.py +++ b/science/config.py @@ -8,9 +8,11 @@ from dataclasses import dataclass from io import BytesIO from pathlib import Path +from textwrap import dedent from typing import BinaryIO from science.build_info import BuildInfo +from science.context import DocConfig, active_context_config from science.data import Data from science.dataclass import Dataclass from science.dataclass.deserializer import parse as parse_dataclass @@ -45,7 +47,7 @@ def parse_config_str(config: str) -> Application: def parse_build_info(data: Data) -> BuildInfo: return BuildInfo.gather( - lift_toml=data.provenance, app_info=data.get_data("app_info", default={}).data + lift_toml=data.provenance, app_info=data.get_data("app_info", default={}, used=True).data ) @@ -102,9 +104,36 @@ def parse_interpreter_group(ig_data: Data) -> InterpreterGroup: interpreters=[interpreters_by_id[Identifier(member)] for member in members], ) - return parse_dataclass( + application = parse_dataclass( lift, Application, interpreters=tuple(interpreters_by_id.values()), custom_parsers={BuildInfo: parse_build_info, InterpreterGroup: parse_interpreter_group}, ) + + unused_items = list(lift.iter_unused_items()) + if unused_items: + doc_config = active_context_config(DocConfig) + doc_url = ( + f"{doc_config.site}/manifest.html" + if doc_config + else "https://science.scie.app/manifest.html" + ) + raise InputError( + dedent( + """\ + The following `lift` manifest entries in {manifest_source} were not recognized: + {unrecognized_fields} + + Refer to the lift manifest format specification at {doc_url} or by running `science doc open manifest`. + """ + ) + .format( + manifest_source=data.provenance.source, + unrecognized_fields="\n".join(key for key, _ in unused_items), + doc_url=doc_url, + ) + .strip() + ) + + return application diff --git a/science/data.py b/science/data.py index e58ff6e..befb508 100644 --- a/science/data.py +++ b/science/data.py @@ -3,11 +3,11 @@ from __future__ import annotations +import dataclasses import os -from collections import OrderedDict from dataclasses import dataclass from enum import Enum, auto -from typing import Any, Collection, Mapping, TypeVar, cast +from typing import Any, Collection, Iterator, Mapping, TypeVar from science.errors import InputError from science.frozendict import FrozenDict @@ -16,11 +16,15 @@ _T = TypeVar("_T") -@dataclass(frozen=True) +@dataclass class Data: provenance: Provenance data: FrozenDict[str, Any] path: str = "" + _unused_data: dict[str, Any] = dataclasses.field(init=False) + + def __post_init__(self) -> None: + self._unused_data = dict(self.data) def config(self, key: str) -> str: return f"`[{self.path}] {key}`" if self.path else f"`[{key}]`" @@ -30,15 +34,20 @@ class Required(Enum): REQUIRED = Required.VALUE - def get_data(self, key: str, default: dict[str, Any] | Required = REQUIRED) -> Data: - data = self.get_value( - key, expected_type=Mapping, default=default # type: ignore[type-abstract] + def get_data( + self, key: str, default: dict[str, Any] | Required = REQUIRED, used: bool = False + ) -> Data: + raw_data = self.get_value( + key, expected_type=Mapping, default=default, used=used # type: ignore[type-abstract] ) - return Data( + data = Data( provenance=self.provenance, - data=FrozenDict(data), + data=FrozenDict(raw_data), path=f"{self.path}.{key}" if self.path else key, ) + if not used: + self._unused_data[key] = data + return data def get_str(self, key: str, default: str | Required = REQUIRED) -> str: return self.get_value(key, expected_type=str, default=default) @@ -57,49 +66,72 @@ def get_list( key: str, expected_item_type: type[_T], default: list[_T] | Required = REQUIRED, + used: bool = True, ) -> list[_T]: value = self.get_value( - key, expected_type=Collection, default=default # type: ignore[type-abstract] - ) - invalid_entries = OrderedDict( - (index, item) - for index, item in enumerate(value, start=1) - if not isinstance(item, expected_item_type) + key, expected_type=Collection, default=default, used=used # type: ignore[type-abstract] ) + + items = [] + invalid_entries = {} + for index, item in enumerate(value, start=1): + if isinstance(item, expected_item_type): + items.append(item) + else: + # As a last resort, see if the item is convertible to the expected_item_type via its constructor. This + # supports Enum and similar types. + try: + items.append(expected_item_type(item)) # type: ignore[call-arg] + except (TypeError, ValueError): + invalid_entries[index] = item + if invalid_entries: invalid_items = [ f"item {index}: {item} of type {self._typename(type(item))}" for index, item in invalid_entries.items() ] + expected_values = "" + if issubclass(expected_item_type, Enum): + expected_values = f" from {{{', '.join(repr(expected.value) for expected in expected_item_type)}}}" + raise InputError( f"Expected {self.config(key)} defined in {self.provenance.source} to be a list " - f"with items of type {self._typename(expected_item_type)} but got " + f"with items of type {self._typename(expected_item_type)}{expected_values} but got " f"{len(invalid_entries)} out of {len(value)} entries of the wrong type:{os.linesep}" f"{os.linesep.join(invalid_items)}" ) - return cast(list[_T], value) + return items def get_data_list( self, key: str, default: list[dict] | Required = REQUIRED, ) -> list[Data]: - return [ + data_list = [ Data( provenance=self.provenance, data=FrozenDict(data), path=f"{self.path}.{key}[{index}]" if self.path else key, ) for index, data in enumerate( - self.get_list(key, expected_item_type=Mapping, default=default), start=1 + self.get_list(key, expected_item_type=Mapping, default=default, used=False), start=1 ) ] + if data_list: + self._unused_data[key] = data_list + return data_list @staticmethod def _typename(type_: type) -> str: return "toml table" if issubclass(type_, Mapping) else type_.__name__ - def get_value(self, key: str, expected_type: type[_T], default: _T | Required = REQUIRED) -> _T: + def get_value( + self, + key: str, + expected_type: type[_T], + default: _T | Required = REQUIRED, + used: bool = True, + ) -> _T: if key not in self.data: if default is self.REQUIRED: raise InputError( @@ -114,7 +146,21 @@ def get_value(self, key: str, expected_type: type[_T], default: _T | Required = f"Expected a {self._typename(expected_type)} for {self.config(key)} but found " f"{value} of type {self._typename(type(value))} in {self.provenance.source}." ) + if used: + self._unused_data.pop(key, None) return value + def iter_unused_items(self) -> Iterator[tuple[str, Any]]: + for key, value in self._unused_data.items(): + if isinstance(value, list) and all(isinstance(item, Data) for item in value): + for index, item in enumerate(value, start=1): + for sub_key, sub_value in item.iter_unused_items(): + yield f"{key}[{index}].{sub_key}", sub_value + elif isinstance(value, Data): + for sub_key, sub_value in value.iter_unused_items(): + yield f"{key}.{sub_key}", sub_value + else: + yield key, value + def __bool__(self): return bool(self.data) diff --git a/science/dataclass/deserializer.py b/science/dataclass/deserializer.py index a97067a..da240ef 100644 --- a/science/dataclass/deserializer.py +++ b/science/dataclass/deserializer.py @@ -54,40 +54,43 @@ def _parse_field( if map_type := type_.issubtype(Mapping): # default is Env - data_value = data.get_data(name, default=cast(dict[str, Any] | Data.Required, default)) + data_value = data.get_data( + name, default=cast(dict[str, Any] | Data.Required, default), used=True + ) # We assume any mapping type used will be constructible with a single dict argument. return cast(_F, map_type(data_value.data)) if type_.issubtype(Collection) and not type_.issubtype(str): item_type = type_.item_type + items: list[Any] = [] if dataclasses.is_dataclass(item_type) or isinstance(item_type, Mapping): data_list = data.get_data_list(name, default=cast(list | Data.Required, default)) + if isinstance(item_type, Mapping): - return cast( - _F, - [ - # We assume any mapping type used will be constructible with a single dict - # argument. - cast(Mapping, item_type(data_item.data)) # type: ignore[misc] - for data_item in data_list - ], + items.extend( + # We assume any mapping type used will be constructible with a single dict + # argument. + cast(_F, item_type(data_item.data)) # type: ignore[misc] + for data_item in data_list ) else: - return cast( - _F, - [ - parse(data_item, item_type, custom_parsers=custom_parsers) - for data_item in data_list - ], + items.extend( + parse(data_item, item_type, custom_parsers=custom_parsers) + for data_item in data_list ) else: - return cast( - _F, + items.extend( data.get_list( name, expected_item_type=item_type, default=cast(list | Data.Required, default) - ), + ) ) + if type_.has_origin_type and (origin_type := type_.origin_type) is not list: + # I.E.: tuple, frozenset, etc. + return origin_type(items) # type: ignore[call-arg] + + return cast(_F, items) + value = data.get_value(name, expected_type=object, default=default) if value is default: return cast(_F, value) @@ -121,7 +124,7 @@ def _parse_field( def parse( - data, + data: Data, data_type: type[_D], *, custom_parsers: Mapping[type, typing.Callable[[Data], Any]] = FrozenDict(), diff --git a/science/exe.py b/science/exe.py index 3f77302..6e8543a 100644 --- a/science/exe.py +++ b/science/exe.py @@ -15,10 +15,11 @@ import textwrap import traceback from io import BytesIO -from pathlib import Path +from pathlib import Path, PurePath from textwrap import dedent from types import TracebackType from typing import BinaryIO +from urllib.parse import urlparse, urlunparse import click import click_log @@ -199,10 +200,22 @@ def _doc(ctx: click.Context, site: str, local: Path | None) -> None: """ ), ) +@click.argument("page", default=None, required=False) @pass_doc -def _open_doc(doc: DocConfig, remote: bool) -> None: - """Opens the local documentation in a browser.""" - click.launch(doc.site if remote else f"file://{doc.local}") +def _open_doc(doc: DocConfig, remote: bool, page: str | None = None) -> None: + """Opens the local documentation in a browser. + + If an optional page argument is supplied, that page will be opened instead of the default doc site page. + """ + url = doc.site if remote else f"file://{doc.local}" + if not page: + if not remote: + url = f"{url}/index.html" + else: + url_info = urlparse(url) + page = f"{page}.html" if not PurePath(page).suffix else page + url = urlunparse(url_info._replace(path=f"{url_info.path}/{page}")) + click.launch(url) @_main.group(cls=DYMGroup, name="provider") diff --git a/science/model.py b/science/model.py index f020262..1248fca 100644 --- a/science/model.py +++ b/science/model.py @@ -487,8 +487,8 @@ class Application(Dataclass): interpreters: tuple[Interpreter, ...] = () interpreter_groups: tuple[InterpreterGroup, ...] = () files: tuple[File, ...] = () - commands: frozenset[Command] = frozenset() - bindings: frozenset[Command] = frozenset() + commands: tuple[Command, ...] = () + bindings: tuple[Command, ...] = () scie_jump: ScieJump | None = None ptex: Ptex | None = None diff --git a/tests/data/interpreter-groups.toml b/tests/data/interpreter-groups.toml index 5ec135b..b7bc6ae 100644 --- a/tests/data/interpreter-groups.toml +++ b/tests/data/interpreter-groups.toml @@ -2,7 +2,7 @@ name = "igs" description = "Test interpreter group selection." -[lift.scie-jump] +[lift.scie_jump] version = "0.11.0" [[lift.interpreters]] diff --git a/tests/data/unrecognized-config-fields.toml b/tests/data/unrecognized-config-fields.toml new file mode 100644 index 0000000..af4841e --- /dev/null +++ b/tests/data/unrecognized-config-fields.toml @@ -0,0 +1,66 @@ +[lift] +name = "igs" +description = "Test interpreter group selection." + +# N.B: `scie-jump` is invalid, should be `scie_jump`. +[lift.scie-jump] +version = "0.11.0" + +[lift.scie_jump] +# N.B: `version2` is invalid, should be `version`. +version2 = "0.11.0" + +[[lift.interpreters]] +id = "cpython310" +provider = "PythonBuildStandalone" +version = "3.10" +lazy = true + +[[lift.interpreters]] +id = "cpython311" +provider = "PythonBuildStandalone" +version = "3.11" +# N.B: `lizzy` is invalid, should be `lazy`. +lizzy = true + +[[lift.interpreter_groups]] +id = "cpython" +selector = "{scie.env.PYTHON}" +members = [ + "cpython310", + "cpython311", +] + +[[lift.commands]] +# N.B: `environ` is invalid. +environ = { key = "value" } +exe = "#{cpython:python}" +args = [ + "-c", + """\ +import json +import sys + + +data = {{ + "version": sys.version_info[:2], + "hash": "{scie.files:hash.#{cpython}}" +} +json.dump(data, sys.stdout) + """, +] +[lift.commands.env] +# N.B: `remove_re2` is invalid, should be `remove_re`. +remove_re2 = [ + "regexp" +] +# N.B: `replace2` is invalid, should be `replace`. +[lift.commands.env.replace2] +KEY = "VALUE" + +[lift.app_info] +key = "value" + +# N.B: `app-info` is invalid, should be `app_info`. +[lift.app-info] +key2 = "value2" \ No newline at end of file diff --git a/tests/test_config.py b/tests/test_config.py index 8664864..56ef952 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -8,6 +8,7 @@ import sys from importlib import resources from pathlib import Path +from textwrap import dedent from science.config import parse_config_file from science.model import Identifier @@ -150,3 +151,38 @@ def test_command_descriptions(tmp_path: Path, science_pyz: Path) -> None: ).stdout ) assert {"": "Print a JSON object of command descriptions by name.", "version": None} == data + + +def test_unrecognized_config_fields(tmp_path: Path, science_pyz: Path) -> None: + with resources.as_file(resources.files("data") / "unrecognized-config-fields.toml") as config: + result = subprocess.run( + args=[ + sys.executable, + str(science_pyz), + "lift", + "build", + "--dest-dir", + str(tmp_path), + config, + ], + text=True, + stderr=subprocess.PIPE, + ) + assert result.returncode != 0 + assert ( + dedent( + f"""\ + The following `lift` manifest entries in {config} were not recognized: + scie-jump + scie_jump.version2 + interpreters[2].lizzy + commands[1].environ + commands[1].env.remove_re2 + commands[1].env.replace2 + app-info + + Refer to the lift manifest format specification at https://science.scie.app/manifest.html or by running `science doc open manifest`. + """ + ) + == result.stderr + )