Merge pull request #8643 from nandahkrishna/livecheck-rubocop

livecheck: add RuboCops

Library/Homebrew/rubocops.rb

@@ -17,6 +17,7 @@ require "rubocops/conflicts"
 require "rubocops/options"
 require "rubocops/urls"
 require "rubocops/lines"
+require "rubocops/livecheck"
 require "rubocops/class"
 require "rubocops/uses_from_macos"
 require "rubocops/files"

Library/Homebrew/rubocops/livecheck.rb

+# frozen_string_literal: true
+
+require "rubocops/extend/formula"
+
+module RuboCop
+  module Cop
+    module FormulaAudit
+      # This cop ensures that no other livecheck information is provided for
+      # skipped formulae.
+      #
+      # @api private
+      class LivecheckSkip < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first
+          return if skip.blank?
+
+          return if find_every_method_call_by_name(livecheck_node).length < 3
+
+          offending_node(livecheck_node)
+          problem "Skipped formulae must not contain other livecheck information."
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            skip = find_every_method_call_by_name(node, :skip).first
+            skip = find_strings(skip).first
+            skip = string_content(skip) if skip.present?
+            corrector.replace(
+              node.source_range,
+              <<~EOS.strip,
+                livecheck do
+                    skip#{" \"#{skip}\"" if skip.present?}
+                  end
+              EOS
+            )
+          end
+        end
+      end
+
+      # This cop ensures that a `url` is specified in the livecheck block.
+      #
+      # @api private
+      class LivecheckUrlProvided < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first
+          return if skip.present?
+
+          livecheck_url = find_every_method_call_by_name(livecheck_node, :url).first
+          return if livecheck_url.present?
+
+          offending_node(livecheck_node)
+          problem "A `url` must be provided to livecheck."
+        end
+      end
+
+      # This cop ensures that a supported symbol (`head`, `stable, `homepage`)
+      # is used when the livecheck `url` is identical to one of these formula URLs.
+      #
+      # @api private
+      class LivecheckUrlSymbol < FormulaCop
+        @offense = nil
+
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first.present?
+          return if skip.present?
+
+          livecheck_url_node = find_every_method_call_by_name(livecheck_node, :url).first
+          livecheck_url = find_strings(livecheck_url_node).first
+          return if livecheck_url.blank?
+
+          livecheck_url = string_content(livecheck_url)
+
+          head = find_every_method_call_by_name(body_node, :head).first
+          head_url = find_strings(head).first
+
+          if head.present? && head_url.blank?
+            head = find_every_method_call_by_name(head, :url).first
+            head_url = find_strings(head).first
+          end
+
+          head_url = string_content(head_url) if head_url.present?
+
+          stable = find_every_method_call_by_name(body_node, :url).first
+          stable_url = find_strings(stable).first
+
+          if stable_url.blank?
+            stable = find_every_method_call_by_name(body_node, :stable).first
+            stable = find_every_method_call_by_name(stable, :url).first
+            stable_url = find_strings(stable).first
+          end
+
+          stable_url = string_content(stable_url) if stable_url.present?
+
+          homepage = find_every_method_call_by_name(body_node, :homepage).first
+          homepage_url = string_content(find_strings(homepage).first) if homepage.present?
+
+          formula_urls = { head: head_url, stable: stable_url, homepage: homepage_url }.compact
+
+          formula_urls.each do |symbol, url|
+            next if url != livecheck_url && url != "#{livecheck_url}/" && "#{url}/" != livecheck_url
+
+            offending_node(livecheck_url_node)
+            @offense = symbol
+            problem "Use `url :#{symbol}`"
+            break
+          end
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrector.replace(node.source_range, "url :#{@offense}")
+            @offense = nil
+          end
+        end
+      end
+
+      # This cop ensures that the `regex` call in the livecheck block uses parentheses.
+      #
+      # @api private
+      class LivecheckRegexParentheses < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first.present?
+          return if skip.present?
+
+          livecheck_regex_node = find_every_method_call_by_name(livecheck_node, :regex).first
+          return if livecheck_regex_node.blank?
+
+          return if parentheses?(livecheck_regex_node)
+
+          offending_node(livecheck_regex_node)
+          problem "The `regex` call should always use parentheses."
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            pattern = node.source.split(" ")[1..].join
+            corrector.replace(node.source_range, "regex(#{pattern})")
+          end
+        end
+      end
+
+      # This cop ensures that the pattern provided to livecheck's `regex` uses `\.t` instead of
+      # `\.tgz`, `\.tar.gz` and variants.
+      #
+      # @api private
+      class LivecheckRegexExtension < FormulaCop
+        TAR_PATTERN = /\\?\.t(ar|(g|l|x)z$|[bz2]{2,4}$)(\\?\.((g|l|x)z)|[bz2]{2,4}|Z)?$/i.freeze
+
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first.present?
+          return if skip.present?
+
+          livecheck_regex_node = find_every_method_call_by_name(livecheck_node, :regex).first
+          return if livecheck_regex_node.blank?
+
+          regex_node = livecheck_regex_node.descendants.first
+          pattern = string_content(find_strings(regex_node).first)
+          match = pattern.match(TAR_PATTERN)
+          return if match.blank?
+
+          offending_node(regex_node)
+          problem "Use `\\.t` instead of `#{match}`"
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            node = find_strings(node).first
+            correct = node.source.gsub(TAR_PATTERN, "\\.t")
+            corrector.replace(node.source_range, correct)
+          end
+        end
+      end
+
+      # This cop ensures that a `regex` is provided when `strategy :page_match` is specified
+      # in the livecheck block.
+      #
+      # @api private
+      class LivecheckRegexIfPageMatch < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first.present?
+          return if skip.present?
+
+          livecheck_strategy_node = find_every_method_call_by_name(livecheck_node, :strategy).first
+          return if livecheck_strategy_node.blank?
+
+          strategy = livecheck_strategy_node.descendants.first.source
+          return if strategy != ":page_match"
+
+          livecheck_regex_node = find_every_method_call_by_name(livecheck_node, :regex).first
+          return if livecheck_regex_node.present?
+
+          offending_node(livecheck_node)
+          problem "A `regex` is required if `strategy :page_match` is present."
+        end
+      end
+
+      # This cop ensures that the `regex` provided to livecheck is case-insensitive,
+      # unless sensitivity is explicitly required for proper matching.
+      #
+      # @api private
+      class LivecheckRegexCaseInsensitive < FormulaCop
+        REGEX_CASE_SENSITIVE_ALLOWLIST = %w[].freeze
+
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          return if REGEX_CASE_SENSITIVE_ALLOWLIST.include?(@formula_name)
+
+          livecheck_node = find_block(body_node, :livecheck)
+          return if livecheck_node.blank?
+
+          skip = find_every_method_call_by_name(livecheck_node, :skip).first.present?
+          return if skip.present?
+
+          livecheck_regex_node = find_every_method_call_by_name(livecheck_node, :regex).first
+          return if livecheck_regex_node.blank?
+
+          regex_node = livecheck_regex_node.descendants.first
+          options_node = regex_node.regopt
+          return if options_node.source.include?("i")
+
+          offending_node(regex_node)
+          problem "Regexes should be case-insensitive unless sensitivity is explicitly required for proper matching."
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            node = node.regopt
+            corrector.replace(node.source_range, "i#{node.source}".chars.sort.join)
+          end
+        end
+      end
+    end
+  end
+end

Library/Homebrew/test/.rubocop_todo.yml

@@ -96,6 +96,7 @@ RSpec/MultipleDescribes:
     - 'rubocops/deprecate_disable_spec.rb'
     - 'rubocops/formula_desc_spec.rb'
     - 'rubocops/lines_spec.rb'
+    - 'rubocops/livecheck_spec.rb'
     - 'rubocops/text_spec.rb'
     - 'rubocops/urls_spec.rb'
     - 'software_spec_spec.rb'

Library/Homebrew/test/rubocops/livecheck_spec.rb

+# frozen_string_literal: true
+
+require "rubocops/livecheck"
+
+describe RuboCop::Cop::FormulaAudit::LivecheckSkip do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when a skipped formula's livecheck block contains other information" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+        ^^^^^^^^^^^^ Skipped formulae must not contain other livecheck information.
+          skip "Not maintained"
+          url :stable
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          skip "Not maintained"
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when a skipped formula's livecheck block contains no other information" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          skip "Not maintained"
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckUrlProvided do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when a `url` is not specified in the livecheck block" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+        ^^^^^^^^^^^^ A `url` must be provided to livecheck.
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when a `url` is specified in the livecheck block" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckUrlSymbol do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when the `url` specified in the livecheck block is identical to a formula URL" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url "https://brew.sh/foo-1.0.tgz"
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `url :stable`
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when the `url` specified in the livecheck block is not identical to a formula URL" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url "https://brew.sh/foo/releases/"
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckRegexParentheses do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when the `regex` call in the livecheck block does not use parentheses" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex %r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `regex` call should always use parentheses.
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when the `regex` call in the livecheck block uses parentheses" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckRegexExtension do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when the `regex` does not use `\\.t` for archive file extensions" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.tgz}i)
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `\\.t` instead of `\\.tgz`
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when the `regex` uses `\\.t` for archive file extensions" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckRegexIfPageMatch do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when there is no `regex` for `strategy :page_match`" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+        ^^^^^^^^^^^^ A `regex` is required if `strategy :page_match` is present.
+          url :stable
+          strategy :page_match
+        end
+      end
+    RUBY
+  end
+
+  it "rreports no offenses when a `regex` is specified for `strategy :page_match`" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          strategy :page_match
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+end
+
+describe RuboCop::Cop::FormulaAudit::LivecheckRegexCaseInsensitive do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when the `regex` is not case-insensitive" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t})
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Regexes should be case-insensitive unless sensitivity is explicitly required for proper matching.
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+
+  it "reports no offenses when the `regex` is case-insensitive" do
+    expect_no_offenses(<<~RUBY)
+      class Foo < Formula
+        url "https://brew.sh/foo-1.0.tgz"
+
+        livecheck do
+          url :stable
+          regex(%r{href=.*?/formula[._-]v?(\\d+(?:\\.\\d+)+)\\.t}i)
+        end
+      end
+    RUBY
+  end
+end