Improve errors for modules without functionality#232
Merged
Conversation
Distinguish two cases when a module has no functionality to render: - No ***functional specs*** header at all: raise the new non-syntax MissingFunctionalitiesError with a clear, actionable message instead of the misleading 'was required but does not contain functional requirements' PlainSyntaxError. - Empty ***functional specs*** section: raise a PlainSyntaxError naming the empty section, instead of silently rendering zero functionalities or reporting a misleading 'no implementation reqs' error. Modules with at least one functionality still flow through the existing implementation-reqs validation.
Split the misnamed check_if_functional_requirements_are_specified (which mutated a caller-passed buffer, raised a validation error, and returned header presence) into three single-purpose helpers: - has_functional_specs_section: pure presence predicate - count_functionalities: functionality count - validate_functionalities_have_implementation_reqs: the impl-reqs validation Reclassify 'imported module must not contain functionalities' from a PlainSyntaxError to the new usage error ImportedModuleWithFunctionalitiesError. The file is syntactically valid and would be fine as a render target; it is only wrong for the import role, so it is a usage error, not a syntax error. This also fixes the prior mislabeling where such a module could surface the misleading 'no implementation reqs' syntax error first. Behavior preserved: missing functional specs (usage), empty functional specs section (syntax), and functionalities-without-impl-reqs (syntax) are unchanged.
pedjaradenkovic
force-pushed
the
fix/handle-modules-without-functionality
branch
from
842c612 to
fc001e5
Compare
NejcS
reviewed
Jul 2, 2026
Comment on lines
+171
to
+180
| def validate_functionalities_have_implementation_reqs(plain_source) -> None: | ||
| """Raise if the module has functionalities but no implementation reqs are specified.""" | ||
| implementation_reqs = plain_source[plain_spec.NON_FUNCTIONAL_REQUIREMENTS] | ||
| has_implementation_reqs = ( | ||
| implementation_reqs is not None | ||
| and hasattr(implementation_reqs, "children") | ||
| and len(implementation_reqs.children) > 0 | ||
| ) | ||
| if not has_implementation_reqs: | ||
| raise PlainSyntaxError("Plain syntax error: functionality with no implementation reqs specified.") |
Contributor
There was a problem hiding this comment.
I think this should be worded better. What this does is it checks if the plain file has any implementation reqs but the comment, function name and error message (at least to me) make it sound like each functionality must have an implementation requirement.
Contributor
Author
There was a problem hiding this comment.
Good point, I'll reword this!
Contributor
|
@pedjaradenkovic in general I think the change is good but I left a small comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fixes: https://github.com/Codeplain-ai/next-microsoft/issues/100
When a
.plainmodule has no functionality to render, the renderer produced misleading errors. This PR fixes those messages and, more importantly, draws a clean line between parse-time (syntax) errors — intrinsic to the file, independent of how it's used — and usage errors — where an otherwise-valid file is wrong for the role it's playing (render target vs. imported dependency).The discriminating test: does the file's validity flip depending on its role? If the same bytes are valid in one role and invalid in another, it's a usage error; if invalid in every role, it's a syntax error.
Error classification
***functional specs***section at allMissingFunctionalitiesError(new)***functional specs***section present but emptyPlainSyntaxErrorPlainSyntaxError(unchanged)ImportedModuleWithFunctionalitiesError(new)Messages
Module <name> does not have any functionality specified. At least one functionality is required for rendering.(noPlain syntax error:prefix — it isn't one)Plain syntax error: Module '<name>' has an empty 'functional specs' section. At least one functionality must be specified.Module <name> is imported but contains functional specs. Imported modules may only provide definitions, implementation reqs, and test reqs.Changes
plain2code_exceptions.py— new user-facing exceptionsMissingFunctionalitiesErrorandImportedModuleWithFunctionalitiesError.plain2code.py— both added toEXPECTED_EXCEPTIONS(reported to the user directly, never sent to Sentry as crashes).plain_file.py— retired the misnamedcheck_if_functional_requirements_are_specified(it mutated a caller-passed buffer, raised a validation error, and returned header presence) and split it into three single-purpose helpers:has_functional_specs_section— pure presence predicatecount_functionalities— functionality countvalidate_functionalities_have_implementation_reqs— the impl-reqs validationThe render path (
plain_file_parser) and the import path (process_imports) now read as the parse-vs-usage layering described above.Notable behavior fix
Reclassifying the import check also fixes prior mislabeling: an imported module that carried functional specs and lacked implementation reqs used to surface the misleading "no implementation reqs" syntax error first. It now reports the correct import usage error regardless of impl reqs.
Testing
test_no_functional_specs_section,test_empty_functional_specs_section(tests/test_plainfileparser.py) andtest_imported_module_with_functionalities(tests/test_imports.py), plus supporting fixtures undertests/data/.test_missing_non_functional_requirements/test_without_non_functional_requirementstill pass (≥1 functionality → preserved impl-reqs check).black/isort/flake8/mypyall clean.--dry-run.