diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index ac8c07e9f1..78e4a92e06 100644 --- a/Library/Homebrew/rubocops.rb +++ b/Library/Homebrew/rubocops.rb @@ -15,3 +15,5 @@ require "rubocops/options" require "rubocops/urls" require "rubocops/lines" require "rubocops/class" + +require "rubocops/rubocop-cask" diff --git a/Library/Homebrew/rubocops/cask/ast/cask_block.rb b/Library/Homebrew/rubocops/cask/ast/cask_block.rb new file mode 100644 index 0000000000..bc8db95905 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/ast/cask_block.rb @@ -0,0 +1,74 @@ +require "forwardable" + +module RuboCop + module Cask + module AST + # This class wraps the AST block node that represents the entire cask + # definition. It includes various helper methods to aid cops in their + # analysis. + class CaskBlock + extend Forwardable + + def initialize(block_node, comments) + @block_node = block_node + @comments = comments + end + + attr_reader :block_node, :comments + + alias cask_node block_node + + def_delegator :cask_node, :block_body, :cask_body + + def header + @header ||= CaskHeader.new(cask_node.method_node) + end + + def stanzas + return [] unless cask_body + + @stanzas ||= cask_body.each_node + .select(&:stanza?) + .map { |node| Stanza.new(node, stanza_comments(node)) } + end + + def toplevel_stanzas + @toplevel_stanzas ||= stanzas.select(&:toplevel_stanza?) + end + + def sorted_toplevel_stanzas + @sorted_toplevel_stanzas ||= sort_stanzas(toplevel_stanzas) + end + + private + + def sort_stanzas(stanzas) + stanzas.sort do |s1, s2| + i1 = stanza_order_index(s1) + i2 = stanza_order_index(s2) + if i1 == i2 + i1 = stanzas.index(s1) + i2 = stanzas.index(s2) + end + i1 - i2 + end + end + + def stanza_order_index(stanza) + Constants::STANZA_ORDER.index(stanza.stanza_name) + end + + def stanza_comments(stanza_node) + stanza_node.each_node.reduce([]) do |comments, node| + comments | comments_hash[node.loc] + end + end + + def comments_hash + @comments_hash ||= Parser::Source::Comment + .associate_locations(cask_node, comments) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/ast/cask_header.rb b/Library/Homebrew/rubocops/cask/ast/cask_header.rb new file mode 100644 index 0000000000..475dd2d8f7 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/ast/cask_header.rb @@ -0,0 +1,43 @@ +module RuboCop + module Cask + module AST + # This class wraps the AST method node that represents the cask header. It + # includes various helper methods to aid cops in their analysis. + class CaskHeader + def initialize(method_node) + @method_node = method_node + end + + attr_reader :method_node + + def dsl_version? + hash_node + end + + def header_str + @header_str ||= source_range.source + end + + def source_range + @source_range ||= method_node.loc.expression + end + + def preferred_header_str + "cask '#{cask_token}'" + end + + def cask_token + @cask_token ||= pair_node.val_node.children.first + end + + def hash_node + @hash_node ||= method_node.each_child_node(:hash).first + end + + def pair_node + @pair_node ||= hash_node.each_child_node(:pair).first + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/ast/stanza.rb b/Library/Homebrew/rubocops/cask/ast/stanza.rb new file mode 100644 index 0000000000..c81e560c22 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/ast/stanza.rb @@ -0,0 +1,66 @@ +require "forwardable" + +module RuboCop + module Cask + module AST + # This class wraps the AST send/block node that encapsulates the method + # call that comprises the stanza. It includes various helper methods to + # aid cops in their analysis. + class Stanza + extend Forwardable + + def initialize(method_node, comments) + @method_node = method_node + @comments = comments + end + + attr_reader :method_node, :comments + + alias stanza_node method_node + + def_delegator :stanza_node, :method_name, :stanza_name + def_delegator :stanza_node, :parent, :parent_node + + def source_range + stanza_node.expression + end + + def source_range_with_comments + comments.reduce(source_range) do |range, comment| + range.join(comment.loc.expression) + end + end + + def_delegator :source_range, :source + def_delegator :source_range_with_comments, :source, + :source_with_comments + + def stanza_group + Constants::STANZA_GROUP_HASH[stanza_name] + end + + def same_group?(other) + stanza_group == other.stanza_group + end + + def toplevel_stanza? + parent_node.cask_block? || parent_node.parent.cask_block? + end + + def ==(other) + self.class == other.class && stanza_node == other.stanza_node + end + + alias eql? == + + Constants::STANZA_ORDER.each do |stanza_name| + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{stanza_name}? + stanza_name == :#{stanza_name} + end + RUBY + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/constants/stanza.rb b/Library/Homebrew/rubocops/cask/constants/stanza.rb new file mode 100644 index 0000000000..0cdfef8ba3 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/constants/stanza.rb @@ -0,0 +1,51 @@ +module RuboCop + module Cask + # Constants available globally for use in all Cask cops. + module Constants + STANZA_GROUPS = [ + [:version, :sha256], + [:url, :appcast, :name, :homepage], + [ + :auto_updates, + :conflicts_with, + :depends_on, + :container, + ], + [ + :suite, + :app, + :pkg, + :installer, + :binary, + :colorpicker, + :dictionary, + :font, + :input_method, + :internet_plugin, + :prefpane, + :qlplugin, + :screen_saver, + :service, + :audio_unit_plugin, + :vst_plugin, + :artifact, + :stage_only, + ], + [:preflight], + [:postflight], + [:uninstall_preflight], + [:uninstall_postflight], + [:uninstall], + [:zap], + [:caveats], + ].freeze + + STANZA_GROUP_HASH = + STANZA_GROUPS.each_with_object({}) do |stanza_group, hash| + stanza_group.each { |stanza| hash[stanza] = stanza_group } + end.freeze + + STANZA_ORDER = STANZA_GROUPS.flatten.freeze + end + end +end diff --git a/Library/Homebrew/rubocops/cask/extend/node.rb b/Library/Homebrew/rubocops/cask/extend/node.rb new file mode 100644 index 0000000000..f8c365d21a --- /dev/null +++ b/Library/Homebrew/rubocops/cask/extend/node.rb @@ -0,0 +1,32 @@ +module RuboCop + module AST + # Extensions for RuboCop's AST Node class + class Node + include RuboCop::Cask::Constants + + def_node_matcher :method_node, "{$(send ...) (block $(send ...) ...)}" + def_node_matcher :block_args, "(block _ $_ _)" + def_node_matcher :block_body, "(block _ _ $_)" + + def_node_matcher :key_node, "{(pair $_ _) (hash (pair $_ _) ...)}" + def_node_matcher :val_node, "{(pair _ $_) (hash (pair _ $_) ...)}" + + def_node_matcher :cask_block?, "(block (send nil? :cask _) args ...)" + + def stanza? + (send_type? || block_type?) && STANZA_ORDER.include?(method_name) + end + + def heredoc? + loc.is_a?(Parser::Source::Map::Heredoc) + end + + def expression + base_expression = loc.expression + descendants.select(&:heredoc?).reduce(base_expression) do |expr, node| + expr.join(node.loc.heredoc_end) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/extend/string.rb b/Library/Homebrew/rubocops/cask/extend/string.rb new file mode 100644 index 0000000000..45d50157dc --- /dev/null +++ b/Library/Homebrew/rubocops/cask/extend/string.rb @@ -0,0 +1,6 @@ +# Utility method extensions for String +class String + def undent + gsub(/^.{#{(slice(/^ +/) || '').length}}/, "") + end +end diff --git a/Library/Homebrew/rubocops/cask/homepage_matches_url.rb b/Library/Homebrew/rubocops/cask/homepage_matches_url.rb new file mode 100644 index 0000000000..4c1315f912 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/homepage_matches_url.rb @@ -0,0 +1,168 @@ +require "forwardable" + +module RuboCop + module Cop + module Cask + # This cop checks that a cask's homepage matches the download url, + # or if it doesn't, checks if a comment in the form + # `# example.com was verified as official when first introduced to the cask` + # is present. + class HomepageMatchesUrl < Cop # rubocop:disable Metrics/ClassLength + extend Forwardable + include CaskHelp + + REFERENCE_URL = + "https://github.com/Homebrew/homebrew-cask/blob/master/doc/" \ + "cask_language_reference/stanzas/url.md#when-url-and-homepage-hostnames-differ-add-a-comment".freeze + + COMMENT_FORMAT = /# [^ ]+ was verified as official when first introduced to the cask/.freeze + + MSG_NO_MATCH = "`%{url}` does not match `%{full_url}`".freeze + + MSG_MISSING = "`%{domain}` does not match `%{homepage}`, a comment has to be added " \ + "above the `url` stanza. For details, see " + REFERENCE_URL + + MSG_WRONG_FORMAT = "`%{comment}` does not match the expected comment format. " \ + "For details, see " + REFERENCE_URL + + MSG_UNNECESSARY = "The URL's domain `%{domain}` matches the homepage `%{homepage}`, " \ + "the comment above the `url` stanza is unnecessary".freeze + + def on_cask(cask_block) + @cask_block = cask_block + return unless homepage_stanza + + add_offenses + end + + private + + attr_reader :cask_block + def_delegators :cask_block, :cask_node, :toplevel_stanzas, + :sorted_toplevel_stanzas + + def add_offenses + toplevel_stanzas.select(&:url?).each do |url| + next if add_offense_unnecessary_comment(url) + next if add_offense_missing_comment(url) + next if add_offense_no_match(url) + next if add_offense_wrong_format(url) + end + end + + def add_offense_unnecessary_comment(stanza) + return unless comment?(stanza) + return unless url_match_homepage?(stanza) + return unless comment_matches_format?(stanza) + return unless comment_matches_url?(stanza) + + comment = comment(stanza).loc.expression + add_offense(comment, + location: comment, + message: format(MSG_UNNECESSARY, domain: domain(stanza), homepage: homepage)) + end + + def add_offense_missing_comment(stanza) + return if url_match_homepage?(stanza) + return if !url_match_homepage?(stanza) && comment?(stanza) + + range = stanza.source_range + url_domain = domain(stanza) + add_offense(range, location: range, message: format(MSG_MISSING, domain: url_domain, homepage: homepage)) + end + + def add_offense_no_match(stanza) + return if url_match_homepage?(stanza) + return unless comment?(stanza) + return if !url_match_homepage?(stanza) && comment_matches_url?(stanza) + + comment = comment(stanza).loc.expression + add_offense(comment, + location: comment, + message: format(MSG_NO_MATCH, url: url_from_comment(stanza), full_url: full_url(stanza))) + end + + def add_offense_wrong_format(stanza) + return if url_match_homepage?(stanza) + return unless comment?(stanza) + return if comment_matches_format?(stanza) + + comment = comment(stanza).loc.expression + add_offense(comment, + location: comment, + message: format(MSG_WRONG_FORMAT, comment: comment(stanza).text)) + end + + def comment?(stanza) + !stanza.comments.empty? + end + + def comment(stanza) + stanza.comments.last + end + + def comment_matches_format?(stanza) + comment(stanza).text =~ COMMENT_FORMAT + end + + def url_from_comment(stanza) + comment(stanza).text + .sub(/[^ ]*# ([^ ]+) .*/, '\1') + end + + def comment_matches_url?(stanza) + full_url(stanza).include?(url_from_comment(stanza)) + end + + def strip_url_scheme(url) + url.sub(%r{^.*://(www\.)?}, "") + end + + def domain(stanza) + strip_url_scheme(extract_url(stanza)).gsub(%r{^([^/]+).*}, '\1') + end + + def extract_url(stanza) + string = stanza.stanza_node.children[2] + return string.str_content if string.str_type? + + string.to_s.gsub(%r{.*"([a-z0-9]+\:\/\/[^"]+)".*}m, '\1') + end + + def url_match_homepage?(stanza) + host = extract_url(stanza).downcase + host_uri = URI(remove_non_ascii(host)) + host = if host.match?(/:\d/) && host_uri.port != 80 + "#{host_uri.host}:#{host_uri.port}" + else + host_uri.host + end + home = homepage.downcase + if (split_host = host.split(".")).length >= 3 + host = split_host[-2..-1].join(".") + end + if (split_home = homepage.split(".")).length >= 3 + home = split_home[-2..-1].join(".") + end + host == home + end + + def full_url(stanza) + strip_url_scheme(extract_url(stanza)) + end + + def homepage + URI(remove_non_ascii(extract_url(homepage_stanza))).host + end + + def homepage_stanza + toplevel_stanzas.find(&:homepage?) + end + + def remove_non_ascii(string) + string.gsub(/\P{ASCII}/, "") + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb b/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb new file mode 100644 index 0000000000..267ecad2ce --- /dev/null +++ b/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb @@ -0,0 +1,38 @@ +require "forwardable" +require "uri" + +module RuboCop + module Cop + module Cask + # This cop checks that a cask's homepage ends with a slash + # if it does not have a path component. + class HomepageUrlTrailingSlash < Cop + include OnHomepageStanza + + MSG_NO_SLASH = "'%{url}' must have a slash after the domain.".freeze + + def on_homepage_stanza(stanza) + url_node = stanza.stanza_node.first_argument + url = url_node.str_content + + return if url !~ %r{^.+://[^/]+$} + + add_offense(url_node, location: :expression, + message: format(MSG_NO_SLASH, url: url)) + end + + def autocorrect(node) + domain = URI(node.str_content).host + + # This also takes URLs like 'https://example.org?path' + # and 'https://example.org#path' into account. + corrected_source = node.source.sub("://#{domain}", "://#{domain}/") + + lambda do |corrector| + corrector.replace(node.source_range, corrected_source) + end + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/mixin/cask_help.rb b/Library/Homebrew/rubocops/cask/mixin/cask_help.rb new file mode 100644 index 0000000000..a8ffea470e --- /dev/null +++ b/Library/Homebrew/rubocops/cask/mixin/cask_help.rb @@ -0,0 +1,18 @@ +module RuboCop + module Cop + module Cask + # Common functionality for cops checking casks + module CaskHelp + def on_block(block_node) + super if defined? super + return unless respond_to?(:on_cask) + return unless block_node.cask_block? + + comments = processed_source.comments + cask_block = RuboCop::Cask::AST::CaskBlock.new(block_node, comments) + on_cask(cask_block) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/mixin/on_homepage_stanza.rb b/Library/Homebrew/rubocops/cask/mixin/on_homepage_stanza.rb new file mode 100644 index 0000000000..0656bcb525 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/mixin/on_homepage_stanza.rb @@ -0,0 +1,25 @@ +module RuboCop + module Cop + module Cask + # Common functionality for checking homepage stanzas. + module OnHomepageStanza + extend Forwardable + include CaskHelp + + def on_cask(cask_block) + @cask_block = cask_block + + toplevel_stanzas.select(&:homepage?).each do |stanza| + on_homepage_stanza(stanza) + end + end + + private + + attr_reader :cask_block + def_delegators :cask_block, + :toplevel_stanzas + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/no_dsl_version.rb b/Library/Homebrew/rubocops/cask/no_dsl_version.rb new file mode 100644 index 0000000000..ba6dc3f2c9 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/no_dsl_version.rb @@ -0,0 +1,62 @@ +require "forwardable" + +module RuboCop + module Cop + module Cask + # Do not use the deprecated DSL version syntax in your cask header. + # + # @example + # # bad + # cask :v1 => 'foo' do + # ... + # end + # + # # good + # cask 'foo' do + # ... + # end + class NoDslVersion < Cop + extend Forwardable + include CaskHelp + + MESSAGE = "Use `%{preferred}` instead of `%{current}`".freeze + + def on_cask(cask_block) + @cask_header = cask_block.header + return unless offense? + + offense + end + + def autocorrect(method_node) + @cask_header = cask_header(method_node) + lambda do |corrector| + corrector.replace(header_range, preferred_header_str) + end + end + + private + + def_delegator :@cask_header, :source_range, :header_range + def_delegators :@cask_header, :header_str, :preferred_header_str + + def cask_header(method_node) + RuboCop::Cask::AST::CaskHeader.new(method_node) + end + + def offense? + @cask_header.dsl_version? + end + + def offense + add_offense(@cask_header.method_node, location: header_range, + message: error_msg) + end + + def error_msg + format(MESSAGE, preferred: preferred_header_str, current: header_str) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/stanza_grouping.rb b/Library/Homebrew/rubocops/cask/stanza_grouping.rb new file mode 100644 index 0000000000..279e3d06b0 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/stanza_grouping.rb @@ -0,0 +1,97 @@ +require "forwardable" + +module RuboCop + module Cop + module Cask + # This cop checks that a cask's stanzas are grouped correctly. + # See https://github.com/Homebrew/homebrew-cask/blob/master/CONTRIBUTING.md#stanza-order + # for more info. + class StanzaGrouping < Cop + extend Forwardable + include CaskHelp + include RangeHelp + + MISSING_LINE_MSG = "stanza groups should be separated by a single " \ + "empty line".freeze + + EXTRA_LINE_MSG = "stanzas within the same group should have no lines " \ + "between them".freeze + + def on_cask(cask_block) + @cask_block = cask_block + @line_ops = {} + add_offenses + end + + def autocorrect(range) + lambda do |corrector| + case line_ops[range.line - 1] + when :insert + corrector.insert_before(range, "\n") + when :remove + corrector.remove(range) + end + end + end + + private + + attr_reader :cask_block, :line_ops + def_delegators :cask_block, :cask_node, :toplevel_stanzas + + def add_offenses + toplevel_stanzas.each_cons(2) do |stanza, next_stanza| + next unless next_stanza + + if missing_line_after?(stanza, next_stanza) + add_offense_missing_line(stanza) + elsif extra_line_after?(stanza, next_stanza) + add_offense_extra_line(stanza) + end + end + end + + def missing_line_after?(stanza, next_stanza) + !(stanza.same_group?(next_stanza) || + empty_line_after?(stanza)) + end + + def extra_line_after?(stanza, next_stanza) + stanza.same_group?(next_stanza) && + empty_line_after?(stanza) + end + + def empty_line_after?(stanza) + source_line_after(stanza).empty? + end + + def source_line_after(stanza) + processed_source[index_of_line_after(stanza)] + end + + def index_of_line_after(stanza) + stanza.source_range.last_line + end + + def add_offense_missing_line(stanza) + line_index = index_of_line_after(stanza) + line_ops[line_index] = :insert + add_offense(line_index, message: MISSING_LINE_MSG) + end + + def add_offense_extra_line(stanza) + line_index = index_of_line_after(stanza) + line_ops[line_index] = :remove + add_offense(line_index, message: EXTRA_LINE_MSG) + end + + def add_offense(line_index, message:) + line_length = [processed_source[line_index].size, 1].max + range = source_range(processed_source.buffer, line_index + 1, 0, + line_length) + super(range, location: range, message: message) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/cask/stanza_order.rb b/Library/Homebrew/rubocops/cask/stanza_order.rb new file mode 100644 index 0000000000..0de2e88b72 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/stanza_order.rb @@ -0,0 +1,53 @@ +require "forwardable" + +module RuboCop + module Cop + module Cask + # This cop checks that a cask's stanzas are ordered correctly. + # See https://github.com/Homebrew/homebrew-cask/blob/master/CONTRIBUTING.md#stanza-order + # for more info. + class StanzaOrder < Cop + extend Forwardable + include CaskHelp + + MESSAGE = "`%{stanza}` stanza out of order".freeze + + def on_cask(cask_block) + @cask_block = cask_block + add_offenses + end + + def autocorrect(stanza) + lambda do |corrector| + correct_stanza_index = toplevel_stanzas.index(stanza) + correct_stanza = sorted_toplevel_stanzas[correct_stanza_index] + corrector.replace(stanza.source_range_with_comments, + correct_stanza.source_with_comments) + end + end + + private + + attr_reader :cask_block + def_delegators :cask_block, :cask_node, :toplevel_stanzas, + :sorted_toplevel_stanzas + + def add_offenses + offending_stanzas.each do |stanza| + message = format(MESSAGE, stanza: stanza.stanza_name) + add_offense(stanza, location: stanza.source_range_with_comments, + message: message) + end + end + + def offending_stanzas + stanza_pairs = toplevel_stanzas.zip(sorted_toplevel_stanzas) + stanza_pairs.each_with_object([]) do |stanza_pair, offending_stanzas| + stanza, sorted_stanza = *stanza_pair + offending_stanzas << stanza unless stanza == sorted_stanza + end + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/rubocop-cask.rb b/Library/Homebrew/rubocops/rubocop-cask.rb index b6e9421b10..f528e59b49 100644 --- a/Library/Homebrew/rubocops/rubocop-cask.rb +++ b/Library/Homebrew/rubocops/rubocop-cask.rb @@ -1,28 +1,16 @@ -require 'rubocop' +require "rubocop" -require 'rubocops/cask/constants/cask_method_names' -require 'rubocops/cask/constants/stanza' +require "rubocops/cask/constants/stanza" -require 'rubocops/cask/ast/stanza' -require 'rubocops/cask/ast/cask_header' -require 'rubocops/cask/ast/cask_block' -require 'rubocops/cask/extend/string' -require 'rubocops/cask/extend/node' -require 'rubocops/cask/mixin/cask_help' -require 'rubocops/cask/mixin/on_homepage_stanza' -require 'rubocops/cask/homepage_matches_url' -require 'rubocops/cask/homepage_url_trailing_slash' -require 'rubocops/cask/no_dsl_version' -require 'rubocops/cask/stanza_order' -require 'rubocops/cask/stanza_grouping' - -module RuboCop - module Cask - DEFAULT_CONFIG = File.expand_path('cask/config/default.yml', __dir__) - end - - ConfigLoader.default_configuration = ConfigLoader.merge_with_default( - ConfigLoader.load_file(Cask::DEFAULT_CONFIG), - Cask::DEFAULT_CONFIG - ) -end +require "rubocops/cask/ast/stanza" +require "rubocops/cask/ast/cask_header" +require "rubocops/cask/ast/cask_block" +require "rubocops/cask/extend/string" +require "rubocops/cask/extend/node" +require "rubocops/cask/mixin/cask_help" +require "rubocops/cask/mixin/on_homepage_stanza" +require "rubocops/cask/homepage_matches_url" +require "rubocops/cask/homepage_url_trailing_slash" +require "rubocops/cask/no_dsl_version" +require "rubocops/cask/stanza_order" +require "rubocops/cask/stanza_grouping" diff --git a/Library/Homebrew/test/rubocops/cask/homepage_matches_url_spec.rb b/Library/Homebrew/test/rubocops/cask/homepage_matches_url_spec.rb new file mode 100644 index 0000000000..36f414a759 --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/homepage_matches_url_spec.rb @@ -0,0 +1,213 @@ +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::HomepageMatchesUrl do + include CaskCop + + subject(:cop) { described_class.new } + + context "when the url matches the homepage and there is no comment" do + let(:source) do + <<-CASK.undent + cask 'foo' do + url 'https://foo.example.com/foo.zip' + homepage 'https://foo.example.com' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url matches the homepage and the url stanza has " \ + "a referrer and no interpolation" do + let(:source) do + <<-CASK.undent + cask 'foo' do + url 'https://foo.example.com/foo.zip', + referrer: 'https://example.com/foo/' + homepage 'https://foo.example.com' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url matches the homepage and the url stanza has " \ + "a referrer and interpolation" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version '1.8.0_72,8.13.0.5' + url "https://foo.example.com/foo-\#{version.after_comma}-\#{version.minor}.\#{version.patch}.\#{version.before_comma.sub(\%r{.*_}, '')}.zip", + referrer: 'https://example.com/foo/' + homepage 'https://foo.example.com' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url matches the homepage but there is a comment " \ + "which does not match the url" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # this is just a comment with information + url 'https://example.com/foo.zip' + homepage 'https://example.com' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the url matches the homepage " \ + "but there is a comment matching the url" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # foo.example.com was verified as official when first introduced to the cask + url 'https://foo.example.com/foo.zip' + homepage 'https://foo.example.com' + end + CASK + end + let(:expected_offenses) do + [{ + message: "The URL's domain `foo.example.com` matches the homepage " \ + "`foo.example.com`, the comment above the `url` stanza is " \ + "unnecessary", + severity: :convention, + line: 2, + column: 2, + source: "# foo.example.com was verified as official when " \ + "first introduced to the cask", + }] + end + + include_examples "reports offenses" + end + + context "when the url does not match the homepage" do + context "when there is a comment matching the url " \ + "but not matching the expected format" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # example.com was verified as official + url 'https://example.com/foo.zip' + homepage 'https://foo.example.org' + end + CASK + end + let(:expected_offenses) do + [{ + message: "`# example.com was verified as official` does not " \ + "match the expected comment format. For details, see " \ + "https://github.com/Homebrew/homebrew-cask/blob/master/doc/" \ + "cask_language_reference/stanzas/url.md#when-url-and-homepage-hostnames-differ-add-a-comment", + severity: :convention, + line: 2, + column: 2, + source: "# example.com was verified as official", + }] + end + + include_examples "reports offenses" + end + + context "when there is a comment matching the url " \ + "and does not have slashes" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # example.com was verified as official when first introduced to the cask + url 'https://example.com/foo.zip' + homepage 'https://foo.example.org' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there is a comment matching the url and has slashes" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # example.com/vendor/app was verified as official when first introduced to the cask + url 'https://downloads.example.com/vendor/app/foo.zip' + homepage 'https://vendor.example.org/app/' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there is a comment which does not match the url" do + let(:source) do + <<-CASK.undent + cask 'foo' do + # example.com was verified as official when first introduced to the cask + url 'https://example.org/foo.zip' + homepage 'https://foo.example.com' + end + CASK + end + let(:expected_offenses) do + [{ + message: "`example.com` does not match `example.org/foo.zip`", + severity: :convention, + line: 2, + column: 2, + source: "# example.com was verified as official when " \ + "first introduced to the cask", + }] + end + + include_examples "reports offenses" + end + + context "when the comment is missing" do + let(:source) do + <<-CASK.undent + cask 'foo' do + url 'https://example.com/foo.zip' + homepage 'https://example.org' + end + CASK + end + let(:expected_offenses) do + [{ + message: "`example.com` does not match `example.org`, a comment " \ + "has to be added above the `url` stanza. For details, see " \ + "https://github.com/Homebrew/homebrew-cask/blob/master/doc/" \ + "cask_language_reference/stanzas/url.md#when-url-and-homepage-hostnames-differ-add-a-comment", + severity: :convention, + line: 2, + column: 2, + source: "url 'https://example.com/foo.zip'", + }] + end + + include_examples "reports offenses" + end + end + + context "when there is no homepage" do + let(:source) do + <<-CASK.undent + cask 'foo' do + url 'https://example.com/foo.zip' + end + CASK + end + + include_examples "does not report any offenses" + end +end diff --git a/Library/Homebrew/test/rubocops/cask/homepage_url_trailing_slash_spec.rb b/Library/Homebrew/test/rubocops/cask/homepage_url_trailing_slash_spec.rb new file mode 100644 index 0000000000..8c11e07ef9 --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/homepage_url_trailing_slash_spec.rb @@ -0,0 +1,63 @@ +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::HomepageUrlTrailingSlash do + include CaskCop + + subject(:cop) { described_class.new } + + context "when the homepage url ends with a slash" do + let(:source) do + <<-CASK.undent + cask 'foo' do + homepage 'https://foo.example.com/' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the homepage url does not end with a slash but has a path" do + let(:source) do + <<-CASK.undent + cask 'foo' do + homepage 'https://foo.example.com/path' + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when the homepage url does not end with a slash and has no path" do + let(:source) do + <<-CASK.undent + cask 'foo' do + homepage 'https://foo.example.com' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + homepage 'https://foo.example.com/' + end + CASK + end + let(:expected_offenses) do + [{ + message: "'https://foo.example.com' must have a slash "\ + "after the domain.", + severity: :convention, + line: 2, + column: 11, + source: "'https://foo.example.com'", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end +end diff --git a/Library/Homebrew/test/rubocops/cask/no_dsl_version_spec.rb b/Library/Homebrew/test/rubocops/cask/no_dsl_version_spec.rb new file mode 100644 index 0000000000..30e32b47f0 --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/no_dsl_version_spec.rb @@ -0,0 +1,36 @@ +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::NoDslVersion do + include CaskCop + + subject(:cop) { described_class.new } + + context "with header method `cask`" do + let(:header_method) { "cask" } + + context "with no dsl version" do + let(:source) { "cask 'foo' do; end" } + + include_examples "does not report any offenses" + end + + context "with dsl version" do + let(:source) { "cask :v1 => 'foo' do; end" } + let(:correct_source) { "cask 'foo' do; end" } + let(:expected_offenses) do + [{ + message: "Use `cask 'foo'` instead of `cask :v1 => 'foo'`", + severity: :convention, + line: 1, + column: 0, + source: "cask :v1 => 'foo'", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + end +end diff --git a/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb b/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb new file mode 100644 index 0000000000..0da2c06b7b --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb @@ -0,0 +1,45 @@ +module CaskCop + shared_examples "does not report any offenses" do + it "does not report any offenses" do + expect_no_offenses(source) + end + end + + shared_examples "reports offenses" do + it "reports offenses" do + expect_reported_offenses(source, expected_offenses) + end + end + + shared_examples "autocorrects source" do + it "autocorrects source" do + expect_autocorrected_source(source, correct_source) + end + end + + def expect_no_offenses(source) + inspect_source(source) + expect(cop.offenses).to be_empty + end + + def expect_reported_offenses(source, expected_offenses) + inspect_source(source) + expect(cop.offenses.size).to eq(expected_offenses.size) + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + def expect_offense(expected, actual) + expect(actual.message).to eq(expected[:message]) + expect(actual.severity).to eq(expected[:severity]) + expect(actual.line).to eq(expected[:line]) + expect(actual.column).to eq(expected[:column]) + expect(actual.location.source).to eq(expected[:source]) + end + + def expect_autocorrected_source(source, correct_source) + new_source = autocorrect_source(source) + expect(new_source).to eq(Array(correct_source).join("\n")) + end +end diff --git a/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb new file mode 100644 index 0000000000..56c8293caa --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb @@ -0,0 +1,300 @@ +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::StanzaGrouping do + include CaskCop + + subject(:cop) { described_class.new } + + let(:missing_line_msg) do + "stanza groups should be separated by a single empty line" + end + let(:extra_line_msg) do + "stanzas within the same group should have no lines between them" + end + + context "when there is only one stanza" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when no stanzas are incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when one stanza is incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: extra_line_msg, + severity: :convention, + line: 3, + column: 0, + source: "\n", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when many stanzas are incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + + name 'Foo' + + homepage 'https://foo.example.com' + + app 'Foo.app' + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kextextension' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + + url 'https://foo.example.com/foo.zip' + name 'Foo' + homepage 'https://foo.example.com' + + app 'Foo.app' + + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kextextension' + end + CASK + end + let(:expected_offenses) do + [{ + message: missing_line_msg, + severity: :convention, + line: 4, + column: 0, + source: " url 'https://foo.example.com/foo.zip'", + }, { + message: extra_line_msg, + severity: :convention, + line: 5, + column: 0, + source: "\n", + }, { + message: extra_line_msg, + severity: :convention, + line: 7, + column: 0, + source: "\n", + }, { + message: missing_line_msg, + severity: :convention, + line: 11, + column: 0, + source: " uninstall :quit => 'com.example.foo',", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when caveats stanza is incorrectly grouped" do + let(:source) do + format(<<-CASK.undent, caveats: caveats.strip) + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + name 'Foo' + app 'Foo.app' + %{caveats} + end + CASK + end + let(:correct_source) do + format(<<-CASK.undent, caveats: caveats.strip) + cask 'foo' do + version :latest + sha256 :no_check + + url 'https://foo.example.com/foo.zip' + name 'Foo' + + app 'Foo.app' + + %{caveats} + end + CASK + end + + context "when caveats is a one-line string" do + let(:caveats) { "caveats 'This is a one-line caveat.'" } + + include_examples "autocorrects source" + end + + context "when caveats is a heredoc" do + let(:caveats) do + <<-CAVEATS.undent + caveats <<-EOS.undent + This is a multiline caveat. + + Let's hope it doesn't cause any problems! + EOS + CAVEATS + end + + include_examples "autocorrects source" + end + + context "when caveats is a block" do + let(:caveats) do + <<-CAVEATS.undent + caveats do + puts 'This is a multiline caveat.' + + puts "Let's hope it doesn't cause any problems!" + end + CAVEATS + end + + include_examples "autocorrects source" + end + end + + context "when the postflight stanza is incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + name 'Foo' + app 'Foo.app' + postflight do + puts 'We have liftoff!' + end + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + + url 'https://foo.example.com/foo.zip' + name 'Foo' + + app 'Foo.app' + + postflight do + puts 'We have liftoff!' + end + end + CASK + end + + include_examples "autocorrects source" + end + + context "when a stanza has a comment" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + url 'https://foo.example.com/foo.zip' + name 'Foo' + app 'Foo.app' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + + url 'https://foo.example.com/foo.zip' + name 'Foo' + + app 'Foo.app' + end + CASK + end + + include_examples "autocorrects source" + end + + # TODO: detect incorrectly grouped stanzas in nested expressions + context "when stanzas are nested in a conditional expression" do + let(:source) do + <<-CASK.undent + cask 'foo' do + if true + version :latest + + sha256 :no_check + end + end + CASK + end + + include_examples "does not report any offenses" + end +end diff --git a/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb b/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb new file mode 100644 index 0000000000..87a417ef5f --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb @@ -0,0 +1,306 @@ +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::StanzaOrder do + include CaskCop + + subject(:cop) { described_class.new } + + context "when there is only one stanza" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when no stanzas are out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when one pair of stanzas is out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + sha256 :no_check + version :latest + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: "`sha256` stanza out of order", + severity: :convention, + line: 2, + column: 2, + source: "sha256 :no_check", + }, { + message: "`version` stanza out of order", + severity: :convention, + line: 3, + column: 2, + source: "version :latest", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when many stanzas are out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + url 'https://foo.example.com/foo.zip' + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kext' + version :latest + app 'Foo.app' + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + app 'Foo.app' + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kext' + end + CASK + end + let(:expected_offenses) do + [{ + message: "`url` stanza out of order", + severity: :convention, + line: 2, + column: 2, + source: "url 'https://foo.example.com/foo.zip'", + }, { + message: "`uninstall` stanza out of order", + severity: :convention, + line: 3, + column: 2, + source: "uninstall :quit => 'com.example.foo',\n" \ + " :kext => 'com.example.foo.kext'", + }, { + message: "`version` stanza out of order", + severity: :convention, + line: 5, + column: 2, + source: "version :latest", + }, { + message: "`sha256` stanza out of order", + severity: :convention, + line: 7, + column: 2, + source: "sha256 :no_check", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when a stanza appears multiple times" do + let(:source) do + <<-CASK.undent + cask 'foo' do + name 'Foo' + url 'https://foo.example.com/foo.zip' + name 'FancyFoo' + version :latest + app 'Foo.app' + sha256 :no_check + name 'FunkyFoo' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + name 'Foo' + name 'FancyFoo' + name 'FunkyFoo' + app 'Foo.app' + end + CASK + end + + it "preserves the original order" do + expect_autocorrected_source(source, correct_source) + end + end + + context "when a stanza has a comment" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + sha256 :no_check # comment on same line + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check # comment on same line + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + end + CASK + end + + include_examples "autocorrects source" + end + + context "when the caveats stanza is out of order" do + let(:source) do + format(<<-CASK.undent, caveats: caveats.strip) + cask 'foo' do + name 'Foo' + url 'https://foo.example.com/foo.zip' + %{caveats} + version :latest + app 'Foo.app' + sha256 :no_check + end + CASK + end + let(:correct_source) do + format(<<-CASK.undent, caveats: caveats.strip) + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + name 'Foo' + app 'Foo.app' + %{caveats} + end + CASK + end + + context "when caveats is a one-line string" do + let(:caveats) { "caveats 'This is a one-line caveat.'" } + + include_examples "autocorrects source" + end + + context "when caveats is a heredoc" do + let(:caveats) do + <<-CAVEATS.undent + caveats <<-EOS.undent + This is a multiline caveat. + + Let's hope it doesn't cause any problems! + EOS + CAVEATS + end + + include_examples "autocorrects source" + end + + context "when caveats is a block" do + let(:caveats) do + <<-CAVEATS.undent + caveats do + puts 'This is a multiline caveat.' + + puts "Let's hope it doesn't cause any problems!" + end + CAVEATS + end + + include_examples "autocorrects source" + end + end + + context "when the postflight stanza is out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + name 'Foo' + url 'https://foo.example.com/foo.zip' + postflight do + puts 'We have liftoff!' + end + version :latest + app 'Foo.app' + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + url 'https://foo.example.com/foo.zip' + name 'Foo' + app 'Foo.app' + postflight do + puts 'We have liftoff!' + end + end + CASK + end + + include_examples "autocorrects source" + end + + # TODO: detect out-of-order stanzas in nested expressions + context "when stanzas are nested in a conditional expression" do + let(:source) do + <<-CASK.undent + cask 'foo' do + if true + sha256 :no_check + version :latest + end + end + CASK + end + + include_examples "does not report any offenses" + end +end