feat(parser): Add #{...} set literal syntax support (Phase 1 of #164)#166
feat(parser): Add #{...} set literal syntax support (Phase 1 of #164)#166andreasronge merged 2 commits intomainfrom
Conversation
Implement Phase 1 of set literal support (issue #164) by adding parser support for Clojure-style `#{...}` set literal syntax. Changes: - Add :set combinator to parser that matches `#{` followed by expressions - Update :expr choice list to include :set before :map_literal (critical for correct precedence - must try `#{` before `{`) - Add build_set/1 helper function to ParserHelpers - Add comprehensive parser tests for empty sets, sets with various element types, whitespace/comma handling, nested sets, and error cases Tests now validate: - Empty set: `#{}` - Sets with numbers, keywords, vectors: `#{1 :a [1 2]}` - Nested sets: `#{#{1 2}}` - Whitespace and commas: `#{ 1 , 2 , 3 }` - Error cases: unclosed sets, space between `#` and `{` All 979 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
PR Review: feat(parser): Add #{...} set literal syntax support (Phase 1 of #164)SummaryThis PR implements Phase 1 (Parser) of the Set Literal epic (#164), adding support for Clojure-style What's Good
Issues (Must Fix)None - the implementation is complete and correct for Phase 1 scope. Suggestions (Optional)
SecurityNo concerns - parser changes only, no external input handling changes. DocumentationNo updates needed for Phase 1. The spec document ( VerdictApprove - The implementation is correct, complete for its stated scope (Phase 1: Parser), follows the specification, and all 979 tests pass. The PR is well-scoped as the first phase of a multi-phase feature. |
|
@claude please fix these issues from PR review: Issue 1: Add test for set containing mapLocation: test "set containing map" do
set_map = "#" <> "{:a {:b 1}}"
assert {:ok, {:set, [{:keyword, :a}, {:map, [{{:keyword, :b}, 1}]}]}} = Parser.parse(set_map)
endIssue 2: Add test for mixed types from specLocation: test "set with mixed types" do
set_mixed = "#" <> "{:a \"b\" 3}"
assert {:ok, {:set, [{:keyword, :a}, {:string, "b"}, 3]}} = Parser.parse(set_mixed)
endPlease fix ALL issues above in a SINGLE commit. Make minimal changes to resolve each issue. |
Auto-Triage SummaryDecisions Made
RationaleBoth suggestions are FIX_NOW because:
Status
The review verdict was Approve, and these are low-risk additions that complete the parser test coverage as specified in the implementation plan. |
github-actions
bot
added
auto-triage-cycle-1
- Add test for set containing map (#{:a {:b 1}})
- Add test for set with mixed types (#{:a "b" 3}) from spec
Addresses PR review suggestions to complete test coverage for parser Phase 1.
🤖 Generated with Claude Code
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Fixed IssuesAll issues from the PR review have been resolved: Issue 1: Set containing map ✅
Issue 2: Set with mixed types ✅
Verification
Commit: b60ed36 |
PR Review: feat(parser): Add #{...} set literal syntax support (Phase 1 of #164)SummaryThis PR implements Phase 1 (Parser) of the Set Literal epic (#164), adding support for Clojure-style What's Good
Issues (Must Fix)None - the implementation is complete and correct for Phase 1 scope. Suggestions (Optional)None - the implementation covers all test cases from the spec (Section 6.1) and follows the existing codebase patterns precisely. SecurityNo concerns - parser changes only, no external input handling changes or new attack vectors. DocumentationNo updates needed for Phase 1. The spec document ( Verification
VerdictApprove - The implementation is correct, complete for its stated scope (Phase 1: Parser), and matches the specification exactly. All 981 tests pass. The PR is well-scoped as the first phase of a multi-phase feature and establishes clean patterns for subsequent phases (analyzer, evaluator, runtime functions, formatter, env bindings, documentation). |
Auto-Triage Summary (Cycle 1 of 3)Review StatusThe PR has already completed a full review and fix cycle:
Decisions MadeNo new issues to triage. The latest review (comment #5) shows:
Action TakenAdded Status
The PR is complete and ready to merge. |
github-actions
bot
added
auto-triage-cycle-2
1 participant
Summary
This implements Phase 1 (Parser) of the Set Literal epic (#164). LLMs commonly emit Clojure-style
#{...}set literal syntax, but PTC-Lisp previously failed to parse it. This PR adds parser support to eliminate a common source of LLM-generated parse errors.Key Changes:
:setcombinator to match#{followed by zero or more expressions followed by}:exprchoice list to include:setBEFORE:map_literalfor correct precedencebuild_set/1helper in ParserHelpersTests Added:
#{}#{1 2 3}#{:a :b}#{#{1 2}}#{[1 2]}#{ 1 , 2 , 3 }#and{All 979 tests passing including 6 new set parser tests.
Closes #165
🤖 Generated with Claude Code