Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 1, 2025

  • Inline syntax-path-element? predicate (just use exact-nonnegative-integer?)
  • Fix test cases for dotted syntax - #'(a . (b c)) is treated as #'(a b c)
  • Add tests for unflatten-improper-list helper function
  • Replace syntax-path-element<=> with natural<=> from rebellion
  • Improve flatten logic to properly handle dotted syntax
  • Distinguish between dotted syntax (flattens to proper list) and truly improper lists (atom tail)
  • Fix syntax-label-paths to preserve original structure while using flattened indices
  • Remove syntax-path-element? definition entirely - use exact-nonnegative-integer? directly
  • Remove syntax-path-element<=> definition entirely - use natural<=> directly
  • Remove redundant test for natural<=> (already tested by Rebellion)
  • All 904 tests pass
Original prompt

This section details on the original issue you should resolve

<issue_title>Get rid of improper syntax paths</issue_title>
<issue_description>They're more trouble than they're worth. The syntax-path API would be much simpler if a syntax path only ever contained a list of integers, and it could be used to traverse syntax objects that contain any kind of datum. For instance, (syntax-ref #'(a b c) (syntax-path (list 0))) and (syntax-ref #'#(a b c) (syntax-path (list 0))) should both return #'a, regardless of whether the parent syntax object contains a list datum or a vector datum. This should work for all of the possible datum types except for hashes, with a caveat for improper lists:

  • Box datums are treated like lists containing one element.
  • Prefab struct datums are treated like lists containing their fields.
  • Improper lists are treated like lists where the improper tail is the last element of the list.
  • Dotted syntax is flattened, e.g. #'(a . (b . (c . d))) is treated the same as #'(a b c . d).

Hash value datums become completely unsupported. That's probably fine: hash literals cause tons of other problems anyway because their value expressions have an undefined and arbitrary ordering that's unrelated to the order they appear in source code.

This might cause various obscure breakages because syntax paths are used extensively in the core source analysis engine of Resyntax. In addition to automated tests, it would be useful to manually test this change by running Resyntax on a large project like Typed Racket or DrRacket.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 1, 2025
Copilot AI changed the title [WIP] Fix improper syntax paths for traversal Simplify syntax-path API to use integer-only indices Nov 1, 2025
Copilot AI requested a review from jackfirth November 1, 2025 17:47
@coveralls
Copy link

coveralls commented Nov 1, 2025

Pull Request Test Coverage Report for Build #105

Details

  • 138 of 178 (77.53%) changed or added relevant lines in 1 file are covered.
  • 93 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.7%) to 92.871%

Changes Missing Coverage Covered Lines Changed/Added Lines %
private/syntax-path.rkt 138 178 77.53%
Files with Coverage Reduction New Missed Lines %
private/syntax-path.rkt 13 81.92%
default-recommendations/analyzers/identifier-usage.rkt 16 93.17%
private/matching-comparator.rkt 64 41.09%
Totals Coverage Status
Change from base Build #100: -0.7%
Covered Lines: 14474
Relevant Lines: 15585

💛 - Coveralls

Copy link
Owner

@jackfirth jackfirth left a comment

Choose a reason for hiding this comment

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

Copilot AI requested a review from jackfirth November 1, 2025 21:31
@jackfirth jackfirth marked this pull request as ready for review November 1, 2025 21:38
Copy link
Owner

@jackfirth jackfirth left a comment

Choose a reason for hiding this comment

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

Copy link
Owner

@jackfirth jackfirth left a comment

Choose a reason for hiding this comment

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

@jackfirth jackfirth merged commit 2fba63c into master Nov 1, 2025
9 checks passed
@jackfirth jackfirth deleted the copilot/fix-improper-syntax-paths branch November 1, 2025 23:55
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.

Get rid of improper syntax paths

3 participants