Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pragma directives not picked up on return statement #540

Merged
merged 4 commits into from
Feb 12, 2025

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Feb 10, 2025

The //dd:span directive is not honored when it's presented on a return statement (where the expression is a literal function expression).

This changes the directive lookup function so it crawls up to the parent if it's a node type that wraps a single expression; effectively:

  • name := <expr> (LHS and RHS)
  • name = <expr> (RHS only)
  • <call> (function portion only)
  • defer <call>
  • <expr> (the expression statement)
  • go <expr>
  • return <expr>
  • ch <- <expr> (the value only)

Fixes #539

@RomainMuller RomainMuller requested a review from a team as a code owner February 10, 2025 11:32
@RomainMuller RomainMuller marked this pull request as draft February 10, 2025 11:34
The `//dd:span` directive is not honored when it's presented on a
`return` statement (where the expression is a literal function
expression).

This changes the directive lookup function so it crawls up to the parent
if it's a node type that wraps a single expression; effectively:
- `name := <expr>` (and other assignment styles)
- `<call>`
- `defer <call>`
- `<expr>` (the expression statement)
- `go <expr>`
- `label: <expr>`
- `return <expr>`
- `ch <- <expr>` (send statement)

Fixes #539
@RomainMuller RomainMuller force-pushed the romain.marcadier/directive-crawl-ups branch from ba5bbf1 to 8e8fd55 Compare February 10, 2025 11:35
@RomainMuller RomainMuller self-assigned this Feb 10, 2025
@RomainMuller RomainMuller marked this pull request as ready for review February 10, 2025 15:54
@RomainMuller RomainMuller force-pushed the romain.marcadier/directive-crawl-ups branch from 15a8078 to c34dde5 Compare February 11, 2025 08:56
@RomainMuller RomainMuller added this pull request to the merge queue Feb 11, 2025
@eliottness eliottness removed this pull request from the merge queue due to a manual request Feb 11, 2025
@RomainMuller RomainMuller added this pull request to the merge queue Feb 12, 2025
Merged via the queue into main with commit e9ef93c Feb 12, 2025
46 checks passed
@RomainMuller RomainMuller deleted the romain.marcadier/directive-crawl-ups branch February 12, 2025 09:30
Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.05%. Comparing base (1ceb738) to head (c5ab2ed).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #540      +/-   ##
==========================================
- Coverage   68.03%   64.05%   -3.98%     
==========================================
  Files         104      106       +2     
  Lines        6736     6917     +181     
==========================================
- Hits         4583     4431     -152     
- Misses       1688     2025     +337     
+ Partials      465      461       -4     
Components Coverage Δ
Generators 46.17% <ø> (ø)
Instruments ∅ <ø> (∅)
Go Driver 73.42% <ø> (-2.48%) ⬇️
Toolexec Driver 54.49% <ø> (-4.01%) ⬇️
Aspects 71.59% <100.00%> (-4.03%) ⬇️
Injector 72.14% <100.00%> (-2.95%) ⬇️
Job Server 65.98% <ø> (-5.12%) ⬇️
Other 64.05% <100.00%> (-3.98%) ⬇️
Files with missing lines Coverage Δ
internal/injector/aspect/join/directive.go 92.10% <100.00%> (-1.65%) ⬇️

... and 35 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants