Skip to content

fix: keep exports of a required module listed before its dependent - #11

Merged
bananaplain merged 3 commits into
mainfrom
fix/keep-exports-of-repeated-required-modules
Oct 12, 2026
Merged

bananaplain merged 3 commits into
mainfrom
fix/keep-exports-of-repeated-required-modules

Conversation

@bananaplain

@bananaplain bananaplain commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

A module whose requires lists an ancestor and then a module that itself requires that ancestor (requires: [a, b], with b requiring a) failed to parse when b uses a concept exported by a:

Syntax error: Concept :A: is not defined in the definitions.

process_required_modules skips a the second time it is reached, so it isn't processed twice. That skip also dropped a's exported definitions, so b was validated without them. Every module above b in the chain failed with the same error.

The skip comes from codeplain, where it is harmless: codeplain validates each required module in a separate, fresh parse, where b processes a itself. Since 0.1.1, plain-parser validates required modules inside the top module's parse, so that plain-parser check matches --dry-run while reading each file once. In that shared walk, the skip loses a's exports.

Change

  • Each module's exported definitions are recorded the first time it is processed and returned again when the module is skipped. Only its own exports, never its ancestors'.
  • No condition changes: diamonds and independent branches are rejected as before. Two shapes now fail with the fixed-order error where 0.1.2 reported a missing concept, because b no longer fails first: requires: [b, c, b], and requires: [base, left, right] with left and right both requiring base. That is the message codeplain gives.
  • One shape is newly rejected, as codeplain rejects it: b defining a concept with the same name as one a exports, with different text, fails with "Concepts were defined multiple times".
  • New fixtures tests/data/requires/shared_export_{base,middle,top,above}.plain cover the module that requires both (top) and a module above it (above).
  • Topology tests: accepted chains, rejected diamonds and independent branches, and two valid chains still rejected (as in codeplain), marked as expected to fail. The cases live in named constants, and the tests around the shared base are named after the modules they parse.

Verification

  • Tests, black, isort, flake8 and mypy pass.
  • plain-parser check across 3,725 example and corpus .plain files: compared with 0.1.2, the only changes are three existing modules with this requires shape, which go from failing to OK, plus the new fixtures.

Release

  • Ships as 0.1.3.
  • plain-forge users on 0.1.2 need uv tool upgrade plain-parser; plain-forge update does not upgrade it.
  • Known difference from codeplain, left for 0.1.4: when more than one module in a chain has an error, plain-parser reports the deepest module's and codeplain the top module's. Accept/reject and the marshalled trees already match.

@bananaplain
bananaplain requested a review from zanjonke October 9, 2026 10:41

@zanjonke zanjonke left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@bananaplain
bananaplain force-pushed the fix/keep-exports-of-repeated-required-modules branch from 8b8d81f to 188f019 Compare October 12, 2026 09:21
Since 0.1.1 every required module is validated inside the parse of the
top module. A module already processed as an earlier entry of the same
requires list is skipped, as in codeplain, but the skip also dropped its
exported definitions, so a dependent listed after it failed with
"Concept ... is not defined". codeplain never saw this because it parses
each required module fresh. The skip now re-adds the module's own exports.
Diamonds and independent branches must fail with the fixed-order
error. Two valid chains the parser still rejects are tests expected
to fail.
The four tests around Top requiring [Base, Middle] with Middle requiring
Base now say which module is parsed and what it must see, and the two
new ones cover the case the fix newly rejects (Middle redefining an
export of Base) and a Base that exports nothing. The rejection tests
now assert PlainSyntaxError, and the topology cases live in named
constants so each group says what it is before its cases.
@bananaplain
bananaplain force-pushed the fix/keep-exports-of-repeated-required-modules branch from 188f019 to 0201607 Compare October 12, 2026 10:03

@hisenb3rg hisenb3rg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bananaplain
bananaplain merged commit d25b8b8 into main Oct 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants