From ef86b67e18478af7a473f9227299de886647baee Mon Sep 17 00:00:00 2001 From: Kenichi Kamiya Date: Wed, 1 Jun 2022 19:45:43 +0900 Subject: [PATCH] [experimental] Enable rbs/steep check for own coading --- Gemfile | 2 +- Rakefile | 9 +++-- lib/ulid.rb | 7 ++-- lib/ulid/monotonic_generator.rb | 18 +++++----- lib/ulid/uuid.rb | 7 ++-- steep_expectations.yml | 58 +++++++++++++++++++++++++++++++++ test/core/test_ulid_class.rb | 14 ++++++-- 7 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 steep_expectations.yml diff --git a/Gemfile b/Gemfile index 3181279f..b25275e4 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,7 @@ end group :development do gem 'debug', '~> 1.5.0', require: false gem 'rbs', '~> 2.5.0', require: false - gem 'steep', require: false + gem 'steep', '~> 1.0.0', require: false gem 'benchmark-ips', '~> 2.10.0', require: false gem 'yard', '~> 0.9.27', require: false gem 'rubocop', '~> 1.30.0', require: false diff --git a/Rakefile b/Rakefile index 2b882750..c8b7ca2e 100644 --- a/Rakefile +++ b/Rakefile @@ -47,7 +47,7 @@ Rake::TestTask.new(:test_longtime) do |tt| end desc 'Signature check, it means `rbs` and `YARD` syntax correctness' -multitask validate_signatures: [:'signature:validate_yard', :'signature:validate_rbs'] +multitask validate_signatures: [:'signature:validate_yard', :'signature:validate_rbs', :'signature:check_false_positive'] desc 'Simulate CI results in local machine as possible' multitask simulate_ci: [:test_all, :validate_signatures, :rubocop] @@ -59,8 +59,13 @@ namespace :signature do end desc 'Check `rbs` definition with `steep`, but it faults from some reasons ref: #26' + task :save_rbs_errors do + sh 'bundle exec steep check --severity-level=warning --save-expectations' + end + + desc 'Check `rbs` definition with `steep`, should be passed at least considering steep_expectations.yml' task :check_false_positive do - sh 'bundle exec steep check --log-level=fatal' + sh 'bundle exec steep check --severity-level=warning --with-expectations' end desc 'Generate YARD docs for the syntax check' diff --git a/lib/ulid.rb b/lib/ulid.rb index 2a269634..19e4d198 100644 --- a/lib/ulid.rb +++ b/lib/ulid.rb @@ -174,10 +174,12 @@ def self.range(period) raise ArgumentError, 'ULID.range takes only `Range[Time]`, `Range[nil]` or `Range[ULID]`' unless Range === period begin_element, end_element, exclude_end = period.begin, period.end, period.exclude_end? + new_begin, new_end = false, false begin_ulid = ( case begin_element when Time + new_begin = true min(begin_element) when nil MIN @@ -191,6 +193,7 @@ def self.range(period) end_ulid = ( case end_element when Time + new_end = true exclude_end ? min(end_element) : max(end_element) when nil exclude_end = false @@ -203,8 +206,8 @@ def self.range(period) end ) - begin_ulid.freeze unless self === begin_element - end_ulid.freeze unless self === end_element + begin_ulid.freeze if new_begin + end_ulid.freeze if new_end Range.new(begin_ulid, end_ulid, exclude_end) end diff --git a/lib/ulid/monotonic_generator.rb b/lib/ulid/monotonic_generator.rb index 55ecb8d4..4341e1a3 100644 --- a/lib/ulid/monotonic_generator.rb +++ b/lib/ulid/monotonic_generator.rb @@ -13,7 +13,7 @@ class MonotonicGenerator undef_method :instance_variable_set def initialize - super() + super @prev = nil end @@ -30,23 +30,25 @@ def inspect # Basically will not happen. Just means this feature prefers error rather than invalid value. def generate(moment: ULID.current_milliseconds) synchronize do - unless @prev - @prev = ULID.generate(moment: moment) - return @prev + current_prev = @prev + unless current_prev + current_prev = ULID.generate(moment: moment) + @prev = current_prev + return current_prev end milliseconds = ULID.milliseconds_from_moment(moment) ulid = ( - if @prev.milliseconds < milliseconds + if current_prev.milliseconds < milliseconds ULID.generate(moment: milliseconds) else - ULID.from_milliseconds_and_entropy(milliseconds: @prev.milliseconds, entropy: @prev.entropy.succ) + ULID.from_milliseconds_and_entropy(milliseconds: current_prev.milliseconds, entropy: current_prev.entropy.succ) end ) - unless ulid > @prev - base_message = "monotonicity broken from unexpected reasons # generated: #{ulid.inspect}, prev: #{@prev.inspect}" + unless ulid > current_prev + base_message = "monotonicity broken from unexpected reasons # generated: #{ulid.inspect}, prev: #{current_prev.inspect}" additional_information = ( if Thread.list == [Thread.main] '# NOTE: looks single thread only exist' diff --git a/lib/ulid/uuid.rb b/lib/ulid/uuid.rb index 4ebdbb84..1c5af70a 100644 --- a/lib/ulid/uuid.rb +++ b/lib/ulid/uuid.rb @@ -32,8 +32,11 @@ def self.from_uuidv4(uuid) def to_uuidv4 # This code referenced https://github.com/ruby/ruby/blob/121fa24a3451b45c41ac0a661b64e9fc8600e589/lib/securerandom.rb#L221-L241 array = octets.pack('C*').unpack('NnnnnN') - array[2] = (array[2] & 0x0fff) | 0x4000 - array[3] = (array[3] & 0x3fff) | 0x8000 + ref2, ref3 = array[2], array[3] + raise unless Integer === ref2 && Integer === ref3 + + array[2] = (ref2 & 0x0fff) | 0x4000 + array[3] = (ref3 & 0x3fff) | 0x8000 ('%08x-%04x-%04x-%04x-%04x%08x' % array).freeze end end diff --git a/steep_expectations.yml b/steep_expectations.yml new file mode 100644 index 00000000..dd5aee84 --- /dev/null +++ b/steep_expectations.yml @@ -0,0 +1,58 @@ +--- +- file: lib/ulid.rb + diagnostics: + - range: + start: + line: 128 + character: 6 + end: + line: 128 + character: 64 + severity: WARNING + message: The branch is unreachable because the condition is exhaustive + code: Ruby::ElseOnExhaustiveCase + - range: + start: + line: 233 + character: 23 + end: + line: 233 + character: 27 + severity: ERROR + message: Type `::Numeric` does not have method `to_i` + code: Ruby::NoMethod + - range: + start: + line: 246 + character: 6 + end: + line: 246 + character: 85 + severity: WARNING + message: The branch is unreachable because the condition is exhaustive + code: Ruby::ElseOnExhaustiveCase + - range: + start: + line: 326 + character: 8 + end: + line: 326 + character: 52 + severity: ERROR + message: |- + UnexpectedError: undefined method `constr' for nil:NilClass + + constr = rhs_result.constr.update_lvar_env do |lvar_env| + ^^^^^^^ + code: Ruby::UnexpectedError + - range: + start: + line: 326 + character: 26 + end: + line: 326 + character: 52 + severity: ERROR + message: sclass receiver must be instance type or singleton type, but type given + `untyped` + code: Ruby::UnsupportedSyntax diff --git a/test/core/test_ulid_class.rb b/test/core/test_ulid_class.rb index 288739f7..b7be7568 100644 --- a/test/core/test_ulid_class.rb +++ b/test/core/test_ulid_class.rb @@ -265,18 +265,25 @@ def test_range assert_equal( range = ULID.min(ULID.floor(time_has_more_value_than_milliseconds1))..ULID.max(ULID.floor(time_has_more_value_than_milliseconds1)), - ULID.range(time_has_more_value_than_milliseconds1..time_has_more_value_than_milliseconds1) + from_time = ULID.range(time_has_more_value_than_milliseconds1..time_has_more_value_than_milliseconds1) ) assert_equal(true, range.cover?(ULID.generate(moment: time_has_more_value_than_milliseconds1))) assert_equal(false, range.cover?(ULID.generate(moment: time_has_more_value_than_milliseconds2))) assert_equal(true, range.cover?(range.begin)) assert_equal(true, range.cover?(range.end)) + assert_false(range.begin.frozen?) + assert_false(range.end.frozen?) + assert_true(from_time.begin.frozen?) + assert_true(from_time.end.frozen?) assert_raises(ArgumentError) do ULID.range end - assert_same(range, ULID.range(range)) + assert_not_same(range, ULID.range(range)) + assert_equal(range, ULID.range(range)) + assert_false(range.begin.frozen?) + assert_false(range.end.frozen?) assert_equal(range.begin..ULID.max, ULID.range(range.begin..nil)) assert_equal(ULID.min..range.end, ULID.range(nil..range.end)) @@ -618,7 +625,8 @@ def test_sample assert_match(/larger than ULID limit.+or negative/, err.message) end - [nil, BasicObject.new, '42', 4.2].each do |evil| + assert_instance_of(ULID, ULID.sample(nil)) + [false, BasicObject.new, '42', 4.2].each do |evil| err = assert_raises(ArgumentError) do ULID.sample(evil) end