Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 1, 2025

The identifier-usage-analyzer was manually traversing all possible fully expanded syntactic forms using extensive syntax-class definitions. This approach required ~250 lines of boilerplate matching every kernel form, making it difficult to maintain compared to variable-mutability-analyzer which uses syntax-traverse and syntax-search.

Changes

  • Removed manual syntax classes: Eliminated fully-expanded-top-level-form, fully-expanded-expression, fully-expanded-formals, and related classes that explicitly matched each kernel form
  • Implemented using syntax-traversal primitives:
    • syntax-traverse labels identifiers with phase information
    • syntax-search finds binding sites (binding-site-identifiers) and usage sites (usage-site-identifiers)
    • Identifiers grouped by phase into the same binding table structure
  • Net reduction: 114 lines (252 removed, 138 added)

The refactored implementation follows the same pattern as variable-mutability-analyzer for consistency.

Before vs After

Before (manual traversal):

(define-syntax-class (fully-expanded-expression phase)
  #:attributes (bound-ids-by-phase used-ids-by-phase)
  #:literal-sets ((kernel-literals #:phase phase))
  
  (pattern id:id
    #:attr bound-ids-by-phase (hash)
    #:attr used-ids-by-phase (hash phase (treelist (attribute id))))
  
  (pattern (let-values ([(id:id ...) (~var rhs (fully-expanded-expression phase))] ...)
             (~var body (fully-expanded-expression phase)) ...+)
    ; ... 10+ more lines per form)
  ; ... repeated for all kernel forms

After (syntax-search):

(define (usage-site-identifiers expanded-stx)
  (let loop ([expanded-stx expanded-stx] [phase 0])
    (syntax-search expanded-stx
      #:literal-sets ([kernel-literals #:phase phase])
      [(define-values (_ ...) rhs) (recur (attribute rhs))]
      [((~or let-values letrec-values) ([(_ ...) rhs] ...) body ...)
       (apply stream-append (append (map recur (attribute rhs))
                                    (map recur (attribute body))))]
      [id:id #:when (identifier? this-syntax) (stream (attribute id))])))
Original prompt

This section details on the original issue you should resolve

<issue_title>Identifier usage analyzer should use syntax-traverse</issue_title>
<issue_description>The implementation of identifier-usage-analyzer is a manual traversal over all possible fully expanded syntactic forms. However, it would be much easier to implement it the same way this logic is implemented in the variable mutability analyzer: as a series of syntax traversal using syntax-search and syntax-traverse.</issue_description>

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


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Nov 1, 2025
Copilot AI changed the title [WIP] Refactor identifier usage analyzer to use syntax-traverse Refactor identifier-usage-analyzer to use syntax-traverse Nov 1, 2025
Copilot AI requested a review from jackfirth November 1, 2025 20:51
@jackfirth jackfirth marked this pull request as ready for review November 1, 2025 20:57
@coveralls
Copy link

Pull Request Test Coverage Report for Build #94

Details

  • 89 of 91 (97.8%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 93.558%

Changes Missing Coverage Covered Lines Changed/Added Lines %
default-recommendations/analyzers/identifier-usage.rkt 89 91 97.8%
Files with Coverage Reduction New Missed Lines %
default-recommendations/analyzers/identifier-usage.rkt 3 95.14%
Totals Coverage Status
Change from base Build #90: 0.02%
Covered Lines: 14421
Relevant Lines: 15414

💛 - Coveralls

@jackfirth jackfirth merged commit ddbc057 into master Nov 1, 2025
9 checks passed
@jackfirth jackfirth deleted the copilot/update-identifier-usage-analyzer branch November 1, 2025 21:44
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.

Identifier usage analyzer should use syntax-traverse

3 participants