diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index a88d0c4c7272916a90bd0d0c7a88c7b37980d85b..76b8d207b8b958c245f2925fe99d6edbc874277e 100644 --- a/Library/Homebrew/rubocops.rb +++ b/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" diff --git a/Library/Homebrew/rubocops/livecheck.rb b/Library/Homebrew/rubocops/livecheck.rb new file mode 100644 index 0000000000000000000000000000000000000000..bfce504e18866ddc48f213381538b60413ef1556 --- /dev/null +++ b/Library/Homebrew/rubocops/livecheck.rb @@ -0,0 +1,245 @@ +# 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 unless skip.present? && find_every_method_call_by_name(livecheck_node).length > 2 + + 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, + "livecheck do\n skip#{" \"#{skip}\"" if skip.present?}\n end", + ) + 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 unless 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 diff --git a/Library/Homebrew/test/.rubocop_todo.yml b/Library/Homebrew/test/.rubocop_todo.yml index 0fbc7c6b0a11d23b1488528b6b3b7bbc6749792f..3474b71ed0b7b4681a6c5d42938614fcd13bc17c 100644 --- a/Library/Homebrew/test/.rubocop_todo.yml +++ b/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' diff --git a/Library/Homebrew/test/rubocops/livecheck_spec.rb b/Library/Homebrew/test/rubocops/livecheck_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5bda3ac2c82e0a98da16a28351d97bd043f66006 --- /dev/null +++ b/Library/Homebrew/test/rubocops/livecheck_spec.rb @@ -0,0 +1,270 @@ +# 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