diff --git a/src/plain_parser/plain_file.py b/src/plain_parser/plain_file.py index 1aaadc9..c4ce1c0 100644 --- a/src/plain_parser/plain_file.py +++ b/src/plain_parser/plain_file.py @@ -709,19 +709,31 @@ def parse_plain_file( ) +@dataclass +class RequiredModule: + """A required module as parsed by the requires walk; validated once the top module is complete.""" + + parse_result: PlainFileParseResult + # Definitions exported to this module by the modules it requires. + ancestor_exported_definitions: list[mistletoe.block_token.token] + code_variables: dict + # Definitions this module exports to the modules that require it. + exported_definitions: list[mistletoe.block_token.token] + # Marshalled plain source tree, set once the module is validated. + tree: dict | None = None + + def process_required_modules( required_modules: list[str], template_dirs: list[str], all_required_modules: list[str], modules_trace: list[str], - chain: list[tuple[str, dict]], - module_exports: dict[str, list[mistletoe.block_token.token]], + required: dict[str, RequiredModule], ) -> list[mistletoe.block_token.token]: - """Parse and fully validate every required module, deepest first. + """Parse every required module, deepest first, without validating it. - Appends ``(module name, marshalled plain source tree)`` to ``chain`` for each module not already in it, - records each module's exported definitions in ``module_exports``, and returns the exported definitions - of the directly required modules. + Records each module in ``required`` the first time it is reached (deepest ancestors first) and returns + the exported definitions of the directly required modules. """ exported_definitions = list[mistletoe.block_token.token]() for module_name in required_modules: @@ -730,7 +742,7 @@ def process_required_modules( if len(all_required_modules) > 0 and module_name == all_required_modules[-1]: # Already processed as an earlier entry; its exports are still visible to this module. - exported_definitions.extend(module_exports[module_name]) + exported_definitions.extend(required[module_name].exported_definitions) continue code_variables: dict = {} @@ -753,8 +765,7 @@ def process_required_modules( template_dirs, all_required_modules, modules_trace + [module_name], - chain, - module_exports, + required, ) own_exported_definitions = list[mistletoe.block_token.token]() @@ -770,20 +781,32 @@ def process_required_modules( exported_concept, plain_file_parse_result.plain_source, renderer ) ) - module_exports[module_name] = own_exported_definitions - exported_definitions.extend(own_exported_definitions) - - marshalled_plain_source = validate_and_marshall_module( - plain_file_parse_result, module_name, ancestor_exported_definitions, code_variables, template_dirs + required[module_name] = RequiredModule( + plain_file_parse_result, ancestor_exported_definitions, code_variables, own_exported_definitions ) - if module_name not in (chain_module_name for chain_module_name, _ in chain): - chain.append((module_name, marshalled_plain_source)) + exported_definitions.extend(own_exported_definitions) all_required_modules.append(module_name) return exported_definitions +def validate_required_modules( + required_modules: list[str], required: dict[str, RequiredModule], template_dirs: list[str] +) -> None: + """Validate recorded required modules top-down, in listed order, each module once.""" + for module_name in required_modules: + module = required[module_name] + if module.tree is not None: + continue + + validate_module_structure(module.parse_result, module_name) + module.tree = validate_and_build_module_tree( + module.parse_result, module_name, module.ancestor_exported_definitions, module.code_variables, template_dirs + ) + validate_required_modules(module.parse_result.required_modules, required, template_dirs) + + def process_exported_definitions(plain_source: dict, exported_definitions: list[mistletoe.block_token.token]) -> None: if len(exported_definitions) == 0: return @@ -806,14 +829,8 @@ def process_exported_definitions(plain_source: dict, exported_definitions: list[ plain_source[plain_spec.DEFINITIONS].children.append(exported_definition) -def validate_and_marshall_module( - plain_file_parse_result: PlainFileParseResult, - module_name: str, - exported_definitions: list[mistletoe.block_token.token], - code_variables: dict, - template_dirs: list[str], -) -> dict: - """Every check a module must pass after parsing, and its marshalled plain source tree.""" +def validate_module_structure(plain_file_parse_result: PlainFileParseResult, module_name: str) -> None: + """Required concepts defined, functional specs present and non-empty, implementation reqs present.""" if len(plain_file_parse_result.required_concepts) > 0: missing_required_concepts_msg = "Missing required concepts: " missing_required_concepts_msg += ", ".join(plain_file_parse_result.required_concepts) @@ -841,6 +858,16 @@ def validate_and_marshall_module( has_requires=bool(plain_file_parse_result.required_modules), ) + +def validate_and_build_module_tree( + plain_file_parse_result: PlainFileParseResult, + module_name: str, + exported_definitions: list[mistletoe.block_token.token], + code_variables: dict, + template_dirs: list[str], +) -> dict: + """Merge exported definitions, validate acceptance tests, linked resources and concepts, and return the + marshalled plain source tree.""" process_exported_definitions(plain_file_parse_result.plain_source, exported_definitions) process_acceptance_tests(plain_file_parse_result.plain_source) @@ -890,20 +917,27 @@ def _parse_module( modules_trace=[], ) - chain: list[tuple[str, dict]] = [] + # Check the top module's structure, walk its requires, finish the top module's checks, then validate + # every required module top-down. + validate_module_structure(plain_file_parse_result, module_name) + + required: dict[str, RequiredModule] = {} exported_definitions = process_required_modules( plain_file_parse_result.required_modules, template_dirs=template_dirs, all_required_modules=[], modules_trace=[], - chain=chain, - module_exports={}, + required=required, ) - marshalled_plain_source = validate_and_marshall_module( + marshalled_plain_source = validate_and_build_module_tree( plain_file_parse_result, module_name, exported_definitions, code_variables, template_dirs ) + validate_required_modules(plain_file_parse_result.required_modules, required, template_dirs) + # The walk records modules deepest first, which is the chain order. + chain = [(chain_module_name, module.tree) for chain_module_name, module in required.items()] + return module_name, marshalled_plain_source, plain_file_parse_result.required_modules, chain diff --git a/tests/test_requires.py b/tests/test_requires.py index e375107..0166b01 100644 --- a/tests/test_requires.py +++ b/tests/test_requires.py @@ -1,7 +1,7 @@ import pytest from plain_parser import plain_file -from plain_parser.exceptions import ModuleDoesNotExistError, PlainSyntaxError +from plain_parser.exceptions import MissingFunctionalitiesError, ModuleDoesNotExistError, PlainSyntaxError def test_non_existent_require(get_test_data_path): @@ -61,6 +61,7 @@ def test_plain_file_parser_validates_required_modules(get_test_data_path): def write_module(directory, module_name, exported_concepts, required_modules, definitions, functional_spec): + # functional_spec=None writes a module without a ***functional specs*** section. exports = "".join(f" - {concept}\n" for concept in exported_concepts) requires = "".join(f" - {required_module}\n" for required_module in required_modules) (directory / f"{module_name}.plain").write_text( @@ -70,8 +71,8 @@ def write_module(directory, module_name, exported_concepts, required_modules, de + "---\n\n" "***definitions***\n\n" + "".join(f"- {definition}\n\n" for definition in definitions) - + "***implementation reqs***\n\n- An implementation requirement.\n\n" - f"***functional specs***\n\n- {functional_spec}\n" + + "***implementation reqs***\n\n- An implementation requirement.\n" + + (f"\n***functional specs***\n\n- {functional_spec}\n" if functional_spec is not None else "") ) @@ -154,6 +155,14 @@ def write_modules_using_required_exports(directory, requires_by_module): marks=pytest.mark.xfail(strict=True, reason="an ancestor must be listed before its dependent"), ), # Top + # || Top lists Base twice + # Base + pytest.param( + {"Top": ["Base", "Base"], "Base": []}, + ["Base", "Top"], + id="top-lists-base-twice", + ), + # Top # | # Upper every module also requires every module below it # | @@ -267,3 +276,95 @@ def test_requires_independent_branches_are_rejected(tmp_path, requires_by_module with pytest.raises(PlainSyntaxError, match="There must be a fixed order how required modules are dependent"): plain_file.parse_module_chain("Top.plain", [str(tmp_path)]) + + +# Two broken modules in one chain: each test pins which error is reported first. +def test_top_module_shape_is_checked_before_its_requires_are_read(tmp_path): + write_module(tmp_path, "Top", [], ["Missing"], [], None) + + with pytest.raises(MissingFunctionalitiesError): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_top_module_shape_is_checked_before_required_modules_are_validated(tmp_path): + write_module(tmp_path, "Base", [], [], [], "Base uses :BaseMissing:.") + write_module(tmp_path, "Top", [], ["Base"], [], None) + + with pytest.raises(MissingFunctionalitiesError): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_parse_errors_of_required_modules_come_before_top_module_concept_errors(tmp_path): + (tmp_path / "Base.plain").write_text( + "***implementation reqs***\n\n- {% if %}\n\n***functional specs***\n\n- Base works.\n" + ) + write_module(tmp_path, "Top", [], ["Base"], [], "Top uses :TopMissing:.") + + with pytest.raises(PlainSyntaxError, match="Invalid Liquid"): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_top_module_concept_errors_come_before_required_module_concept_errors(tmp_path): + write_module(tmp_path, "Base", [], [], [], "Base uses :BaseMissing:.") + write_module(tmp_path, "Top", [], ["Base"], [], "Top uses :TopMissing:.") + + with pytest.raises(PlainSyntaxError, match="Concept :TopMissing: is not defined"): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_required_modules_are_validated_top_down(tmp_path): + write_module(tmp_path, "Base", [], [], [], "Base uses :BaseMissing:.") + write_module(tmp_path, "Middle", [], ["Base"], [], "Middle uses :MiddleMissing:.") + write_module(tmp_path, "Top", [], ["Middle"], [], "Top works.") + + with pytest.raises(PlainSyntaxError, match="Concept :MiddleMissing: is not defined"): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_module_listed_twice_around_its_dependent_is_rejected(tmp_path): + write_modules_using_required_exports(tmp_path, {"Top": ["Base", "Middle", "Base"], "Middle": ["Base"], "Base": []}) + + with pytest.raises(PlainSyntaxError, match="There must be a fixed order how required modules are dependent"): + plain_file.parse_module_chain("Top.plain", [str(tmp_path)]) + + +def test_chain_has_a_tree_for_every_module_reached_by_the_walk(tmp_path): + write_modules_using_required_exports(tmp_path, {"Top": ["Base", "Middle"], "Middle": ["Base"], "Base": []}) + + chain = plain_file.parse_module_chain("Top.plain", [str(tmp_path)]) + + assert [module_name for module_name, _ in chain] == ["Base", "Middle", "Top"] + assert all("functional specs" in tree for _, tree in chain) + + +def test_required_module_without_functional_specs_is_rejected(tmp_path): + write_module(tmp_path, "Base", [], [], [], None) + write_module(tmp_path, "Top", [], ["Base"], [], "Top works.") + + with pytest.raises(MissingFunctionalitiesError, match="Module 'Base'"): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_top_module_concept_errors_come_before_required_module_structure_errors(tmp_path): + write_module(tmp_path, "Base", [], [], [], None) + write_module(tmp_path, "Top", [], ["Base"], [], "Top uses :TopMissing:.") + + with pytest.raises(PlainSyntaxError, match="Concept :TopMissing: is not defined"): + plain_file.plain_file_parser("Top.plain", [str(tmp_path)]) + + +def test_each_required_module_is_validated_once_top_down(tmp_path, monkeypatch): + write_modules_using_required_exports(tmp_path, {"Top": ["Base", "Middle"], "Middle": ["Base"], "Base": []}) + # A second validation pass over a module would be idempotent, so only the call count can observe it. + built = [] + build_tree = plain_file.validate_and_build_module_tree + + def record_and_build(parse_result, module_name, *args): + built.append(module_name) + return build_tree(parse_result, module_name, *args) + + monkeypatch.setattr(plain_file, "validate_and_build_module_tree", record_and_build) + + plain_file.parse_module_chain("Top.plain", [str(tmp_path)]) + + assert built == ["Top", "Base", "Middle"]