Populate declaration signature comments, close coverage gaps, fix round-trip bug - #1
Merged
Merged
Conversation
openscad_cpp_evaluator's `use <file>` statement resolution needs to build one Scope over declaration nodes drawn from more than one independently- owned AST vector at once (the current file's own nodes plus another file's already-parsed declarations) -- not expressible as a single vector<unique_ptr<ASTNode>>. Both new overloads share their existing unique_ptr-vector counterpart's implementation; the owning overloads are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pretty_print.cpp: 56.09% -> 95.11% line coverage. Real-syntax tests for every previously-missing formatting branch: every binary/unary operator, bare let/echo/assert/function-literal expressions, PrimaryCall/Index/Member, range-literal step, every list-comprehension clause kind (short/long c-style for headers, ListCompLet vs. bare LetOp-as-list-element, nested comprehensions, if/if-else/each), multiline wrapping for for/intersection_for/let/echo/assert/function/module declarations, ternary edge cases, standalone top-level comments/blank lines, and inline comments across ~25 different AST constructs. inline_comment_attach.cpp: 58.00% -> 89.79% line coverage. This port's classifyNode needs an explicit switch over every NodeKind (no runtime reflection like the Python reference's dataclasses.fields() walk), so nearly every operator/clause/statement kind needed its own dedicated comment-attachment test: named arguments, ListCompLet, ModularIfElse, comment-free sibling subtrees, use-statement fallback, and the already-consumed/out-of-range comment-skipping paths that only show up across multiple statements sharing one comment list. Two real findings along the way, documented via tests rather than fixed (out of scope for a coverage pass): a pre-existing round-trip quirk where multiple leading line-comments on separate call arguments don't reparse to the same attachment, and a more notable one -- ParameterDeclaration's and FunctionDeclaration/ModuleDeclaration's leading/pre-name/post-name/ post-params comment fields are declared, serialized, and rendered, but nothing in this port's comment-attachment pipeline ever populates them; a comment written next to a parameter or declaration name silently migrates to become a leading comment on the body instead. A handful of defensive fallback branches remain uncovered: unlike the Python reference, this port's pretty-printer/comment-attach helpers live in file-local anonymous namespaces, so -- unlike testing Python's private functions directly -- no other translation unit can call them to exercise those paths without weakening the encapsulation. 613 tests pass, up from 563. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… fields
fmtMultilineArgsGeneric now moves a leading `//` comment onto the previous
line instead of giving it a fresh one, so re-parsing no longer misclassifies
it as standalone and loses its attachment (was affecting all 6 call sites
that share this function: call args, echo/assert args, function/module
params).
Also implements claimDeclSignatureComments/attachDeclarationComments, which
populate FunctionDeclaration/ModuleDeclaration's preNameComments/
postNameComments/postParamsComments and ParameterDeclaration's
leadingComments/trailingComments -- fields that were declared, serialized,
and rendered but never actually populated by the comment-attachment
pipeline. Recovers the parameter list's '(' / ')' positions (not captured
by the grammar) via a raw-source scan that skips already-extracted comment
spans.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
buildScopes/collectHoistedDeclarations(needed byopenscad_cpp_evaluator'suse <file>resolution, which builds oneScopeover declaration nodes drawn from more than one independently-owned AST vector at once)pretty_print.cppcoverage (56.09% → 95.11%) andinline_comment_attach.cppcoverage (58.00% → 89.79%); 613 tests passing, up from 563fmtMultilineArgsGenericused to print a leading//comment on its own fresh line, which re-parsing then misclassified as standalone (losing its attachment) — now moves it onto the previous line instead, matching how list-literal formatting already handled the same situationpreNameComments/postNameComments/postParamsComments(FunctionDeclaration/ModuleDeclaration) andParameterDeclaration::leadingComments/trailingComments— these were declared, serialized, and rendered but never actually populated by the comment-attachment pipeline, so a comment near a function/module's name or parameter list silently migrated onto the body instead. Ports the same fix made in theopenscad_lalr_parserPython sibling; verified both produce byte-identical output on the same combined-gap-kinds test script.Test plan
attachDeclarationComments/claimDeclSignatureCommentscross-checked byte-for-byte against the Python port's own fix🤖 Generated with Claude Code