Skip to content

Conversation

erezrokah
Copy link
Contributor

This started as a bug fix the Walk function. It skips traversing constraints if Right is nil

if !Walk(n.Right, fn) {

if node == nil || !fn(node) {

This PR tries a more generic approach to prevent this bug and others by returning true if the node is nil.
I also added a test that fails without this fix

@coveralls
Copy link

Pull Request Test Coverage Report for Build 17773387987

Details

  • 177 of 330 (53.64%) changed or added relevant lines in 1 file are covered.
  • 162 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.4%) to 50.065%

Changes Missing Coverage Covered Lines Changed/Added Lines %
parser/walk.go 177 330 53.64%
Files with Coverage Reduction New Missed Lines %
parser/walk.go 162 41.55%
Totals Coverage Status
Change from base Build 17635045137: -0.4%
Covered Lines: 7292
Relevant Lines: 14565

💛 - Coveralls

@git-hulk git-hulk changed the title fix: Don't stop on nil values when traversing the AST Fix shouldn't stop on nil values when traversing the AST Sep 17, 2025
@git-hulk git-hulk merged commit 1893845 into AfterShip:master Sep 17, 2025
2 checks passed
@git-hulk
Copy link
Member

@erezrokah Thanks for your great fix.

@erezrokah erezrokah deleted the fix/dont_stop_walk_nil branch September 18, 2025 17:03
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