From 2d86a91025e3600c0e9176253738bc509ad62eb9 Mon Sep 17 00:00:00 2001 From: Emily Samp Date: Fri, 2 Feb 2024 09:55:18 -0600 Subject: [PATCH] wip --- config/default.yml | 2 +- .../forbid_overlapping_and_annotations.rb | 79 +++++++++++++++++++ .../no_overlapping_and_annotations.rb | 71 ----------------- .../rbi_version_annotation_helper.rb | 27 +++++++ .../valid_version_annotations.rb | 14 +--- lib/rubocop/cop/sorbet_cops.rb | 3 +- ...forbid_overlapping_and_annotations_spec.rb | 49 ++++++++++++ .../no_overlapping_and_annotations_spec.rb | 21 ----- 8 files changed, 162 insertions(+), 104 deletions(-) create mode 100644 lib/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations.rb delete mode 100644 lib/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations.rb create mode 100644 lib/rubocop/cop/sorbet/rbi_versioning/rbi_version_annotation_helper.rb create mode 100644 spec/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations_spec.rb delete mode 100644 spec/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations_spec.rb diff --git a/config/default.yml b/config/default.yml index a1001e22..58e390f6 100644 --- a/config/default.yml +++ b/config/default.yml @@ -301,7 +301,7 @@ Sorbet/ValidVersionAnnotations: Include: - "**/*.rbi" -Sorbet/NoOverlappingAndAnnotations: +Sorbet/ForbidOverlappingAndAnnotations: Description: 'Ensures any gem version annotations using "and" operation do not overlap unnecessarily.' Enabled: pending VersionAdded: '<>' diff --git a/lib/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations.rb b/lib/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations.rb new file mode 100644 index 00000000..23844970 --- /dev/null +++ b/lib/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Sorbet + # Checks that RBI gem version annotations do not contain "and" statements with + # overlapping gem versions. + # + # @example + # # bad + # # @version > 3.2.3, > 4.5 + # + # # bad + # # @version > 1.0, <= 2.0, = 1.5 + # + # # good + # # @version < 1.0, >= 2.0 + # + class ForbidOverlappingAndAnnotations < Base + include RBIVersionAnnotationHelper + + MSG = "Some message here" + + def on_new_investigation + rbi_version_annotations.each do |comment| + ranges = [] + versions(comment).each do |version| + ranges += version_to_ranges(version) + end + + overlap = ranges.combination(2).any? do |range_1, range_2| + range_1.cover?(range_2) || range_2.cover?(range_1) + end + + add_offense(comment) if overlap + end + end + + private + + def version_to_ranges(version_string) + operator, version = version_string.strip.split(" ") + # TODO: make more specific + raise if operator.nil? || version.nil? + + begin + gem_version = Gem::Version.new(version) + rescue ArgumentError + # TODO: do something here + return + end + + case operator + when "=" + [Range.new(gem_version, gem_version)] + when "!=" + prev_version_down = Gem::Version.new(gem_version.to_s + "-pre") + next_version_up = Gem::Version.new(gem_version.to_s + ".1") + [Range.new(nil, prev_version_down), Range.new(next_version_up, nil)] + when ">" + next_version_up = Gem::Version.new(gem_version.to_s + ".1") + [Range.new(next_version_up, nil)] + when ">=" + [Range.new(gem_version, nil)] + when "<" + [Range.new(nil, gem_version, true)] # exclude ending value + when "<=" + [Range.new(nil, gem_version)] # include ending value + when "~>" + [Range.new(gem_version, gem_version.bump)] + else + # TODO: make more specific + raise "INVALID OPERATOR" + end + end + end + end + end +end diff --git a/lib/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations.rb b/lib/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations.rb deleted file mode 100644 index 152711a3..00000000 --- a/lib/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - module Sorbet - # TODO: Write cop description and example of bad / good code. For every - # `SupportedStyle` and unique configuration, there needs to be examples. - # Examples must have valid Ruby syntax. Do not use upticks. - # - # @safety - # Delete this section if the cop is not unsafe (`Safe: false` or - # `SafeAutoCorrect: false`), or use it to explain how the cop is - # unsafe. - # - # @example EnforcedStyle: bar (default) - # # Description of the `bar` style. - # - # # bad - # bad_bar_method - # - # # bad - # bad_bar_method(args) - # - # # good - # good_bar_method - # - # # good - # good_bar_method(args) - # - # @example EnforcedStyle: foo - # # Description of the `foo` style. - # - # # bad - # bad_foo_method - # - # # bad - # bad_foo_method(args) - # - # # good - # good_foo_method - # - # # good - # good_foo_method(args) - # - class NoOverlappingAndAnnotations < Base - # TODO: Implement the cop in here. - # - # In many cases, you can use a node matcher for matching node pattern. - # See https://github.com/rubocop/rubocop-ast/blob/master/lib/rubocop/ast/node_pattern.rb - # - # For example - MSG = 'Use `#good_method` instead of `#bad_method`.' - - # TODO: Don't call `on_send` unless the method name is in this list - # If you don't need `on_send` in the cop you created, remove it. - RESTRICT_ON_SEND = %i[bad_method].freeze - - # @!method bad_method?(node) - def_node_matcher :bad_method?, <<~PATTERN - (send nil? :bad_method ...) - PATTERN - - def on_send(node) - return unless bad_method?(node) - - add_offense(node) - end - end - end - end -end diff --git a/lib/rubocop/cop/sorbet/rbi_versioning/rbi_version_annotation_helper.rb b/lib/rubocop/cop/sorbet/rbi_versioning/rbi_version_annotation_helper.rb new file mode 100644 index 00000000..ab491ca0 --- /dev/null +++ b/lib/rubocop/cop/sorbet/rbi_versioning/rbi_version_annotation_helper.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Sorbet + module RBIVersionAnnotationHelper + VERSION_PREFIX = "# @version " + + def rbi_version_annotations + processed_source.comments.select do |comment| + version_annotation?(comment) + end + end + + private + + def version_annotation?(comment) + comment.text.start_with?(VERSION_PREFIX) + end + + def versions(comment) + comment.text.delete_prefix(VERSION_PREFIX).split(", ") + end + end + end + end +end diff --git a/lib/rubocop/cop/sorbet/rbi_versioning/valid_version_annotations.rb b/lib/rubocop/cop/sorbet/rbi_versioning/valid_version_annotations.rb index 911980d2..9ebbed47 100644 --- a/lib/rubocop/cop/sorbet/rbi_versioning/valid_version_annotations.rb +++ b/lib/rubocop/cop/sorbet/rbi_versioning/valid_version_annotations.rb @@ -19,17 +19,15 @@ module Sorbet # # @version <= 4.3-preview # class ValidVersionAnnotations < Base - MSG = "Invalid gem version(s) detected: %s" + include RBIVersionAnnotationHelper - VERSION_PREFIX = "# @version " + MSG = "Invalid gem version(s) detected: %s" def on_new_investigation - processed_source.comments.each_with_index do |comment, _comment_idx| - next unless version_annotation?(comment) - + rbi_version_annotations.each do |comment| invalid_versions = [] - comment.text.delete_prefix(VERSION_PREFIX).split(", ").each do |version| + versions.each do |version| invalid_versions << version unless valid_version?(version) end @@ -42,10 +40,6 @@ def on_new_investigation private - def version_annotation?(comment) - comment.text.start_with?(VERSION_PREFIX) - end - def valid_version?(version_string) parts = version_string.strip.split(" ") return false unless parts.length == 2 diff --git a/lib/rubocop/cop/sorbet_cops.rb b/lib/rubocop/cop/sorbet_cops.rb index c253a1f5..9c977afb 100644 --- a/lib/rubocop/cop/sorbet_cops.rb +++ b/lib/rubocop/cop/sorbet_cops.rb @@ -24,8 +24,9 @@ require_relative "sorbet/rbi/forbid_rbi_outside_of_allowed_paths" require_relative "sorbet/rbi/single_line_rbi_class_module_definitions" +require_relative "sorbet/rbi_versioning/rbi_version_annotation_helper" require_relative "sorbet/rbi_versioning/valid_version_annotations" -require_relative "sorbet/rbi_versioning/no_overlapping_and_annotations" +require_relative "sorbet/rbi_versioning/forbid_overlapping_and_annotations" require_relative "sorbet/signatures/allow_incompatible_override" require_relative "sorbet/signatures/checked_true_in_signature" diff --git a/spec/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations_spec.rb b/spec/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations_spec.rb new file mode 100644 index 00000000..ba90c983 --- /dev/null +++ b/spec/rubocop/cop/sorbet/rbi_versioning/forbid_overlapping_and_annotations_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +RSpec.describe(RuboCop::Cop::Sorbet::ForbidOverlappingAndAnnotations, :config) do + it "does not register an offense when comment is not a version annotation" do + expect_no_offenses(<<~RUBY) + # a random comment + RUBY + end + + it "does not register an offense when versions do not overlap" do + expect_no_offenses(<<~RUBY) + # @version < 1.0, >= 2.0 + RUBY + end + + it "registers an offense when one version is contained within a range" do + expect_offense(<<~RUBY) + # @version < 1.0, = 0.4.0 + ^^^^^^^^^^^^^^^^^^^^^^^^^ Some message here + RUBY + end + + it "registers an offense when one version is contained within one of a few ranges" do + expect_offense(<<~RUBY) + # @version > 5.0, < 1.0, = 0.4 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Some message here + RUBY + end + + it "does not register an offense when one version partially overlaps with another" do + expect_no_offenses(<<~RUBY) + # @version > 5.0, < 7.0 + RUBY + end + + it "registers an offense when twiddle-waka operator is used" do + expect_offense(<<~RUBY) + # @version ~> 3.6, = 3.6.8 + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Some message here + RUBY + end + + it "registers an offense when not-equal operator is used" do + expect_offense(<<~RUBY) + # @version != 4.0, <= 3.8 + ^^^^^^^^^^^^^^^^^^^^^^^^^ Some message here + RUBY + end +end diff --git a/spec/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations_spec.rb b/spec/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations_spec.rb deleted file mode 100644 index 20b64d86..00000000 --- a/spec/rubocop/cop/sorbet/rbi_versioning/no_overlapping_and_annotations_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::Sorbet::NoOverlappingAndAnnotations, :config do - let(:config) { RuboCop::Config.new } - - # TODO: Write test code - # - # For example - it 'registers an offense when using `#bad_method`' do - expect_offense(<<~RUBY) - bad_method - ^^^^^^^^^^ Use `#good_method` instead of `#bad_method`. - RUBY - end - - it 'does not register an offense when using `#good_method`' do - expect_no_offenses(<<~RUBY) - good_method - RUBY - end -end