Skip to content

fix: Fix potential NME when converting ParameterType to envelope#1789

Merged
luke-hill merged 1 commit intocucumber:mainfrom
mitchgrout:1788-fix-nme-for-parameter-type
Apr 9, 2026
Merged

fix: Fix potential NME when converting ParameterType to envelope#1789
luke-hill merged 1 commit intocucumber:mainfrom
mitchgrout:1788-fix-nme-for-parameter-type

Conversation

@mitchgrout
Copy link
Copy Markdown
Contributor

Description

Sets source_reference to nil if the parameter type transformer does not provide a valid source location, i.e. is nil.

Fixes #1788

Type of change

Bug fix (non-breaking change which fixes an issue)

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

  • Tests have been added for any changes to behaviour of the code
  • New and existing tests are passing locally and on CI
  • bundle exec rubocop reports no offenses
  • RDoc comments have been updated
  • CHANGELOG.md has been updated

@mitchgrout
Copy link
Copy Markdown
Contributor Author

@luke-hill relatively straight-forward fix, although I'm not super pleased with the rspec test due to the method of interest being private. I don't think this warrants an update to any of the features since it's an admittedly weird way to declare a ParameterType(), but I can add one if you think it's worth it

Copy link
Copy Markdown
Contributor

@luke-hill luke-hill left a comment

Choose a reason for hiding this comment

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

Not sure I agree on no source location being provided when a bound method. because it still exists there.

But I've not dug into it in more detail

Also we need to keep the contract up. I might ask how the other flavours deal with it

Comment thread CHANGELOG.md Outdated
Comment thread lib/cucumber/glue/registry_and_more.rb
Comment thread spec/cucumber/glue/registry_and_more_spec.rb Outdated
Comment thread spec/cucumber/glue/registry_and_more_spec.rb Outdated
@luke-hill
Copy link
Copy Markdown
Contributor

FYI I just encountered this at my job today. We used a lambda autofix from rubocop

Comment thread spec/cucumber/glue/registry_and_more_spec.rb
@luke-hill
Copy link
Copy Markdown
Contributor

Few style tweaks but other than that this is good to merge soon @mitchgrout

@luke-hill
Copy link
Copy Markdown
Contributor

Note to self. Rebase with main (Should fix CCK issues), fix up CR style items and merge in

@luke-hill
Copy link
Copy Markdown
Contributor

Hi @mitchgrout are you around and able to fix this? If not I'll try pick it up this month

@luke-hill
Copy link
Copy Markdown
Contributor

Hi @mitchgrout can you rebase this and fix up the review points. It will make v11 (Which should be out in a month or less hopefully, if I can get the last bits of fixes for CCK in)

@mitchgrout mitchgrout force-pushed the 1788-fix-nme-for-parameter-type branch from 7e305fa to 6cc7723 Compare March 31, 2026 08:19
@mitchgrout mitchgrout changed the title bugfix: Fix potential NME when converting ParameterType to envelope fix: Fix potential NME when converting ParameterType to envelope Mar 31, 2026
@mitchgrout mitchgrout force-pushed the 1788-fix-nme-for-parameter-type branch from 6cc7723 to 38af403 Compare March 31, 2026 08:25
@mitchgrout mitchgrout requested a review from luke-hill March 31, 2026 08:26
@luke-hill
Copy link
Copy Markdown
Contributor

@mitchgrout thanks for this. Looks pretty good.

Next steps is you'll need to wait a short time (Probably a few days), until I merge a big PR into main.

At which point you'll need to rebase. And then CI should be fully green.

NB: At this point you'll need a fully green build - alongside rubocop compliance

@luke-hill
Copy link
Copy Markdown
Contributor

@mitchgrout this is provisionally good.

Can you rebase / merge with latest main. At which point a CI run should be fully green.

Thankyou for your patience. I will endeavour to get this released soon in cucumber v11

@mitchgrout
Copy link
Copy Markdown
Contributor Author

No problem - sorry for the delay on my side. I'm overseas for the next 2 days, so will update after then.

@mitchgrout mitchgrout force-pushed the 1788-fix-nme-for-parameter-type branch from 38af403 to 9856dcd Compare April 9, 2026 03:05
@mitchgrout
Copy link
Copy Markdown
Contributor Author

Rebased and checked locally (Win10 setup w/ Ruby 3.2.2) - some seemingly expected failures in unrelated specs, but nothing that seems related to this change.

Copy link
Copy Markdown
Contributor

@luke-hill luke-hill left a comment

Choose a reason for hiding this comment

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

Thankyou for your patience. This will be released as part of v11 (Coming soon)

@luke-hill luke-hill merged commit 24f0615 into cucumber:main Apr 9, 2026
15 checks passed
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.

NoMethodError when method(sym) used for ParameterType.transformer

2 participants