Skip to content

Fix usage of internal @check macro#1179

Merged
nickrobinson251 merged 4 commits intomainfrom
npr-check-io-undef
Feb 18, 2026
Merged

Fix usage of internal @check macro#1179
nickrobinson251 merged 4 commits intomainfrom
npr-check-io-undef

Conversation

@nickrobinson251
Copy link
Collaborator

Fixes an undefvar error (found by JET.jl on a package that calls CSV.write(file::String, table::NamedTuple):

│││││││││││┌ writebom(buf::Vector{UInt8}, pos::Any, len::Int64) @ CSV ~/.julia/packages/CSV/XLcqT/src/write.jl:306
││││││││││││ `CSV.io` is not defined: CSV.io

Repro:

julia> using CSV; CSV.writebom(UInt8[1,2,3], 2, 1)
ERROR: UndefVarError: `io` not defined
Stacktrace:
 [1] writebom(buf::Vector{UInt8}, pos::Int64, len::Int64)
   @ CSV ~/repos/CSV.jl/src/write.jl:311
 [2] top-level scope
   @ REPL[2]:1

the @check macro assumes certain variables are in scope, namely io (and buf, pos, len), but that wasn't true for writebom.

this PR updates writebom to take a variable named io, and updates @check to assert a io variable is defined.
the other variables are always referenced in the expanded code, so we don't @assert @isdefined those.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an UndefVarError in the writebom function caused by the internal @check macro assuming an io variable is in scope when it wasn't. The fix updates writebom to accept an io parameter and adds a compile-time assertion to the @check macro to catch similar issues in the future.

Changes:

  • Updated writebom function signature to include io parameter
  • Added assertion in @check macro to verify io is defined in caller's scope
  • Updated all writebom call sites to pass the io argument
  • Added test case that exercises the fixed code path

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/write.jl Updated writebom signature to accept io parameter, added assertion to @check macro, and updated all call sites
test/write.jl Added test case for writebom function that exercises the bug fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (7819c85) to head (2fe45af).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1179      +/-   ##
==========================================
+ Coverage   90.56%   90.65%   +0.09%     
==========================================
  Files           9        9              
  Lines        2321     2322       +1     
==========================================
+ Hits         2102     2105       +3     
+ Misses        219      217       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nickrobinson251 nickrobinson251 enabled auto-merge (squash) February 18, 2026 12:16
@nickrobinson251 nickrobinson251 merged commit 9ab387e into main Feb 18, 2026
10 of 11 checks passed
@nickrobinson251 nickrobinson251 deleted the npr-check-io-undef branch February 18, 2026 12:45
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