Skip to content

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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ refactored to use newer, less procedural ruby_)
- Fixed an issue where a change to one example in compatibility testing wasn't fully adhered to ([luke-hill](https://github.com/luke-hill))
- Fixed Ruby 3.4+ issue where error backtraces weren't being formatted. ([#1771](https://github.com/cucumber/cucumber-ruby/pull/1771) [orien](https://github.com/orien))
- Fix some problematic specs that were leaking state and showcasing an issue on JRuby ([#1783](https://github.com/cucumber/cucumber-ruby/pull/1783) [luke-hill](https://github.com/luke-hill))
- Fixed an issue where NoMethodError could be raised when declaring a parameter-type that used bound methods ([#1789](https://github.com/cucumber/cucumber-ruby/pull/1789))
Copy link
Contributor

Choose a reason for hiding this comment

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

This will conflict soon as I'm about to cut v10. I will happily cut another v10.0.1 soon after if desired


### Removed
- `StepDefinitionLight` associated methods. The class itself is present but deprecated
Expand Down
16 changes: 12 additions & 4 deletions lib/cucumber/glue/registry_and_more.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,22 @@ def parameter_type_envelope(parameter_type)
regular_expressions: parameter_type.regexps.map(&:to_s),
prefer_for_regular_expression_match: parameter_type.prefer_for_regexp_match,
use_for_snippets: parameter_type.use_for_snippets,
source_reference: Cucumber::Messages::SourceReference.new(
uri: parameter_type.transformer.source_location[0],
location: Cucumber::Messages::Location.new(line: parameter_type.transformer.source_location[1])
)
source_reference: source_reference_for(parameter_type.transformer)
)
)
end

def source_reference_for(transformer)
# #source_location may return nil if no definition was found
# This is the case for transformers created using method(sym) or similar
return nil if transformer.source_location.nil?

Cucumber::Messages::SourceReference.new(
uri: transformer.source_location[0],
location: Cucumber::Messages::Location.new(line: transformer.source_location[1])
)
end

def hooks
@hooks ||= Hash.new { |h, k| h[k] = [] }
end
Expand Down
56 changes: 56 additions & 0 deletions spec/cucumber/glue/registry_and_more_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'spec_helper'
require 'cucumber/glue/registry_and_more'
require 'cucumber/cucumber_expressions/parameter_type'
require 'support/fake_objects'

module Cucumber
Expand Down Expand Up @@ -221,5 +222,60 @@ class << registry.current_world
end
end
end

describe RegistryAndMore do
let(:registry) { described_class.new(double, configuration) }
let(:configuration) { Configuration.new({}) }

describe '#parameter_type_envelope' do
subject(:envelope) { registry.send(:parameter_type_envelope, parameter_type) }

let(:parameter_type) { CucumberExpressions::ParameterType.new(name, regexp, type, transformer, use_for_snippets, prefer_for_regexp_match) }
let(:id) { '279e0f28-c91b-4de2-89c0-e7fbc2a15406' }
let(:name) { 'person' }
let(:regexp) { /"[^"]+"/ }
let(:type) { String }
let(:transformer) { ->(s) { s } }
let(:use_for_snippets) { false }
let(:prefer_for_regexp_match) { true }

before do
allow(configuration).to receive_message_chain(:id_generator, :new_id).and_return(id) # rubocop:disable RSpec/MessageChain
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to remove inline stuff, I don't mind it being added to the overall directive to be fixed up at a later date or if you can tackle it here that would be good

end

it 'produces an envelope with the expected contents' do
expect(envelope).to be_a Cucumber::Messages::Envelope
expect(envelope.parameter_type).to be_a Cucumber::Messages::ParameterType
expect(envelope.parameter_type).to have_attributes(
id: '279e0f28-c91b-4de2-89c0-e7fbc2a15406',
name: 'person',
regular_expressions: [%("[^"]+")],
prefer_for_regular_expression_match: true,
use_for_snippets: false,
source_reference: anything # tested in later cases
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid cop offenses can this be returned as something like :no_op

)
end

context 'when provided a ParameterType with transformer being a lambda' do
let(:transformer) { ->(s) { s } }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think is autocorrected via rubocop to lambda(&:to_s) as an example


it 'includes the lambda source-location in the envelope' do
expect(envelope.parameter_type.source_reference).to be_a Cucumber::Messages::SourceReference
expect(envelope.parameter_type.source_reference).to have_attributes(
uri: transformer.source_location[0],
location: have_attributes(line: transformer.source_location[1])
)
end
end

context 'when provided a ParameterType with transformer being a bound method, which has no source location' do
let(:transformer) { String.method(:new) }

it 'does not include the source-location in the envelope' do
expect(envelope.parameter_type.source_reference).to be_nil
end
end
end
end
end
end