#include? is more liberal about compared types #251
Conversation
updated #include? to be more liberal in what it excepts. It can make comparisons with instances of compatible IPAddr, Nexpose::IPRange, and strings that can be coerced into either of those classes. Old behavior related to returning false for incompatible strings has been preserved with warnings emitted to stderr.
| end | ||
|
|
||
| context 'when IPRange spans multiple addresses' do | ||
| let( :iprange ) { Nexpose::IPRange.new('192.168.1.64','192.168.1.95') } |
There was a problem hiding this comment.
Space inside parentheses detected.
Space missing after comma.
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(below_subject, equivalent) | ||
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(equivalent, above_subject) | ||
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(below_subject, above_subject) | ||
|
|
| end | ||
|
|
||
| context 'when IPRange spans multiple addresses' do | ||
| let( :iprange ) { Nexpose::IPRange.new('192.168.1.64','192.168.1.95') } |
There was a problem hiding this comment.
Space inside parentheses detected.
Space missing after comma.
| end | ||
|
|
||
| context 'when IPRange contains a single address' do | ||
| let( :iprange ) { Nexpose::IPRange.new('192.168.1.81') } |
There was a problem hiding this comment.
Space inside parentheses detected.
Unnecessary spacing detected.
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(below_subject, equivalent) | ||
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(equivalent, above_subject) | ||
| it_behaves_like 'uncovered address', Nexpose::IPRange.new(below_subject, above_subject) | ||
|
|
|
|
||
| shared_examples_for 'incompatible type' do |other| | ||
| it 'raises an ArgumentError' do | ||
| expect{ iprange.include?(other) }.to raise_error( ArgumentError, /incompatible type/) |
There was a problem hiding this comment.
Space missing to the left of {.
Space inside parentheses detected.
| end | ||
|
|
||
| context 'when IPRange contains a single address' do | ||
| let( :iprange ) { Nexpose::IPRange.new('192.168.1.81') } |
There was a problem hiding this comment.
Space inside parentheses detected.
Unnecessary spacing detected.
| end | ||
|
|
||
| it 'emits a warning to stderr' do | ||
| expect{ iprange.include?(unusable_string) }.to output(/could not coerce/).to_stderr |
|
|
||
| shared_examples_for 'incompatible type' do |other| | ||
| it 'raises an ArgumentError' do | ||
| expect{ iprange.include?(other) }.to raise_error( ArgumentError, /incompatible type/) |
There was a problem hiding this comment.
Space missing to the left of {.
Space inside parentheses detected.
| end | ||
|
|
||
| it 'traps exceptions from IPAddr.initialize' do | ||
| expect{ iprange.include?(unusable_string) }.not_to raise_error |
| end | ||
|
|
||
| it 'emits a warning to stderr' do | ||
| expect{ iprange.include?(unusable_string) }.to output(/could not coerce/).to_stderr |
| end | ||
| end | ||
| describe '#include?' do | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body beginning.
| end | ||
|
|
||
| it 'traps exceptions from IPAddr.initialize' do | ||
| expect{ iprange.include?(unusable_string) }.not_to raise_error |
| @@ -0,0 +1,267 @@ | |||
| require 'spec_helper' | |||
There was a problem hiding this comment.
Missing frozen string literal comment.
| end | ||
| end | ||
| describe '#include?' do | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body beginning.
| end | ||
| end | ||
| end | ||
| end |
| @@ -0,0 +1,267 @@ | |||
| require 'spec_helper' | |||
There was a problem hiding this comment.
Missing frozen string literal comment.
| (ip_from <= other_from) && (other_from <= ip_to) | ||
| else | ||
| ip_from = IPAddr.new(self.from) | ||
| ip_to = IPAddr.new(self.to) |
| end | ||
| end | ||
| end | ||
| end |
| other_from = IPAddr.new(other.from) | ||
| (ip_from <= other_from) && (other_from <= ip_to) | ||
| else | ||
| ip_from = IPAddr.new(self.from) |
| (ip_from <= other_from) && (other_from <= ip_to) | ||
| else | ||
| ip_from = IPAddr.new(self.from) | ||
| ip_to = IPAddr.new(self.to) |
| false | ||
| elsif (other.to==nil) && (self.to!=nil) | ||
| ip_from = IPAddr.new(self.from) | ||
| ip_to = IPAddr.new(self.to) |
| other_from = IPAddr.new(other.from) | ||
| (ip_from <= other_from) && (other_from <= ip_to) | ||
| else | ||
| ip_from = IPAddr.new(self.from) |
| elsif (other.to!=nil) && (self.to==nil) | ||
| false | ||
| elsif (other.to==nil) && (self.to!=nil) | ||
| ip_from = IPAddr.new(self.from) |
| false | ||
| elsif (other.to==nil) && (self.to!=nil) | ||
| ip_from = IPAddr.new(self.from) | ||
| ip_to = IPAddr.new(self.to) |
| eql?(other) | ||
| elsif (other.to!=nil) && (self.to==nil) | ||
| false | ||
| elsif (other.to==nil) && (self.to!=nil) |
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
Redundant self detected.
Prefer !expression.nil? over expression != nil.
Surrounding space missing for operator !=.
| elsif (other.to!=nil) && (self.to==nil) | ||
| false | ||
| elsif (other.to==nil) && (self.to!=nil) | ||
| ip_from = IPAddr.new(self.from) |
| def include_iprange?(other) | ||
| if (other.to==nil) && (self.to==nil) | ||
| eql?(other) | ||
| elsif (other.to!=nil) && (self.to==nil) |
There was a problem hiding this comment.
Prefer !expression.nil? over expression != nil.
Surrounding space missing for operator !=.
Redundant self detected.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
| eql?(other) | ||
| elsif (other.to!=nil) && (self.to==nil) | ||
| false | ||
| elsif (other.to==nil) && (self.to!=nil) |
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
Redundant self detected.
Prefer !expression.nil? over expression != nil.
Surrounding space missing for operator !=.
| def include_iprange?(other) | ||
| if (other.to==nil) && (self.to==nil) | ||
| eql?(other) | ||
| elsif (other.to!=nil) && (self.to==nil) |
There was a problem hiding this comment.
Prefer !expression.nil? over expression != nil.
Surrounding space missing for operator !=.
Redundant self detected.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
| end | ||
|
|
||
| def include_iprange?(other) | ||
| if (other.to==nil) && (self.to==nil) |
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
Redundant self detected.
| end | ||
|
|
||
| def include_iprange?(other) | ||
| if (other.to==nil) && (self.to==nil) |
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
Surrounding space missing for operator ==.
Redundant self detected.
|
finished simplifying/consolidating unit tests, but it looks like i've got some style cleanup to do. I'll close this one and resubmit as a single squashed commit. |
The #include? method now can accept Strings, IPAddr, and Nexpose::IPRange objects when testing for inclusion.
Description
Comparisons have been delegated to private methods #include_iprange? and include_ipaddr? (which passes through to include_iprange?). Care has been taken to preserve old behavior and trap runtime exceptions for compared strings that cannot be coerced into a compatible type. The method will raise an exception if an attempt is made to compare with an incompatible type.
Motivation and Context
This enhancement will behave more correctly in more obvious ways for comparisons to commonly used data types. IPRange has also been extracted from
site.rbinto its own separate file.How Has This Been Tested?
Existing unit tests pass
New unit tests have been extracted to
spec/nexpose/ip_range_spec.rb:Finished in 0.05846 seconds (files took 0.52899 seconds to load)
93 examples, 0 failures
Types of changes
Checklist: