Apply suggestions from review comments
- Rename `strictish` to `strict_only` in `add_error` method.
- Return just `errors`, a Set, not `{ errors: errors }`, a Hash,
from `Auditor.audit`.
This commit is contained in:
parent
53a17b921f
commit
d636d2de37
@ -81,10 +81,10 @@ module Cask
|
|||||||
!errors?
|
!errors?
|
||||||
end
|
end
|
||||||
|
|
||||||
sig { params(message: T.nilable(String), location: T.nilable(String), strictish: T::Boolean).void }
|
sig { params(message: T.nilable(String), location: T.nilable(String), strict_only: T::Boolean).void }
|
||||||
def add_error(message, location: nil, strictish: false)
|
def add_error(message, location: nil, strict_only: false)
|
||||||
# Only raise non-critical audits if the user specified `--strict`.
|
# Only raise non-critical audits if the user specified `--strict`.
|
||||||
return if strictish && !@strict
|
return if strict_only && !@strict
|
||||||
|
|
||||||
errors << ({ message: message, location: location })
|
errors << ({ message: message, location: location })
|
||||||
end
|
end
|
||||||
@ -192,7 +192,7 @@ module Cask
|
|||||||
# increases the maintenance burden.
|
# increases the maintenance burden.
|
||||||
return if cask.tap == "homebrew/cask-fonts"
|
return if cask.tap == "homebrew/cask-fonts"
|
||||||
|
|
||||||
add_error("Cask should have a description. Please add a `desc` stanza.", strictish: true) if cask.desc.blank?
|
add_error("Cask should have a description. Please add a `desc` stanza.", strict_only: true) if cask.desc.blank?
|
||||||
end
|
end
|
||||||
|
|
||||||
sig { void }
|
sig { void }
|
||||||
@ -382,7 +382,7 @@ module Cask
|
|||||||
|
|
||||||
add_error(
|
add_error(
|
||||||
"possible duplicate, cask token conflicts with Homebrew core formula: #{Formatter.url(core_formula_url)}",
|
"possible duplicate, cask token conflicts with Homebrew core formula: #{Formatter.url(core_formula_url)}",
|
||||||
strictish: true,
|
strict_only: true,
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -417,19 +417,19 @@ module Cask
|
|||||||
add_error "cask token contains version designation '#{match_data[:designation]}'"
|
add_error "cask token contains version designation '#{match_data[:designation]}'"
|
||||||
end
|
end
|
||||||
|
|
||||||
add_error("cask token mentions launcher", strictish: true) if token.end_with? "launcher"
|
add_error("cask token mentions launcher", strict_only: true) if token.end_with? "launcher"
|
||||||
|
|
||||||
add_error("cask token mentions desktop", strictish: true) if token.end_with? "desktop"
|
add_error("cask token mentions desktop", strict_only: true) if token.end_with? "desktop"
|
||||||
|
|
||||||
add_error("cask token mentions platform", strictish: true) if token.end_with? "mac", "osx", "macos"
|
add_error("cask token mentions platform", strict_only: true) if token.end_with? "mac", "osx", "macos"
|
||||||
|
|
||||||
add_error("cask token mentions architecture", strictish: true) if token.end_with? "x86", "32_bit", "x86_64",
|
add_error("cask token mentions architecture", strict_only: true) if token.end_with? "x86", "32_bit", "x86_64",
|
||||||
"64_bit"
|
"64_bit"
|
||||||
|
|
||||||
frameworks = %w[cocoa qt gtk wx java]
|
frameworks = %w[cocoa qt gtk wx java]
|
||||||
return if frameworks.include?(token) || !token.end_with?(*frameworks)
|
return if frameworks.include?(token) || !token.end_with?(*frameworks)
|
||||||
|
|
||||||
add_error("cask token mentions framework", strictish: true)
|
add_error("cask token mentions framework", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
sig { void }
|
sig { void }
|
||||||
@ -451,7 +451,7 @@ module Cask
|
|||||||
|
|
||||||
add_error(
|
add_error(
|
||||||
"Download does not require additional version components. Use `&:short_version` in the livecheck",
|
"Download does not require additional version components. Use `&:short_version` in the livecheck",
|
||||||
strictish: true,
|
strict_only: true,
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -496,7 +496,7 @@ module Cask
|
|||||||
"#{message} fix the signature of their app."
|
"#{message} fix the signature of their app."
|
||||||
end
|
end
|
||||||
|
|
||||||
add_error(message, strictish: true)
|
add_error(message, strict_only: true)
|
||||||
when Artifact::Pkg
|
when Artifact::Pkg
|
||||||
path = downloaded_path
|
path = downloaded_path
|
||||||
next unless path.exist?
|
next unless path.exist?
|
||||||
@ -504,7 +504,7 @@ module Cask
|
|||||||
result = system_command("pkgutil", args: ["--check-signature", path], print_stderr: false)
|
result = system_command("pkgutil", args: ["--check-signature", path], print_stderr: false)
|
||||||
|
|
||||||
unless result.success?
|
unless result.success?
|
||||||
add_error(<<~EOS, strictish: true)
|
add_error(<<~EOS, strict_only: true)
|
||||||
Signature verification failed:
|
Signature verification failed:
|
||||||
#{result.merged_output}
|
#{result.merged_output}
|
||||||
macOS on ARM requires applications to be signed.
|
macOS on ARM requires applications to be signed.
|
||||||
@ -516,7 +516,7 @@ module Cask
|
|||||||
result = system_command("stapler", args: ["validate", path], print_stderr: false)
|
result = system_command("stapler", args: ["validate", path], print_stderr: false)
|
||||||
next if result.success?
|
next if result.success?
|
||||||
|
|
||||||
add_error(<<~EOS, strictish: true)
|
add_error(<<~EOS, strict_only: true)
|
||||||
Signature verification failed:
|
Signature verification failed:
|
||||||
#{result.merged_output}
|
#{result.merged_output}
|
||||||
macOS on ARM requires applications to be signed.
|
macOS on ARM requires applications to be signed.
|
||||||
@ -643,7 +643,7 @@ module Cask
|
|||||||
return if metadata.nil?
|
return if metadata.nil?
|
||||||
return unless metadata["archived"]
|
return unless metadata["archived"]
|
||||||
|
|
||||||
add_error("GitHub repo is archived", strictish: cask.discontinued?)
|
add_error("GitHub repo is archived", strict_only: cask.discontinued?)
|
||||||
end
|
end
|
||||||
|
|
||||||
sig { void }
|
sig { void }
|
||||||
@ -657,7 +657,7 @@ module Cask
|
|||||||
return if metadata.nil?
|
return if metadata.nil?
|
||||||
return unless metadata["archived"]
|
return unless metadata["archived"]
|
||||||
|
|
||||||
add_error("GitLab repo is archived", strictish: cask.discontinued?)
|
add_error("GitLab repo is archived", strict_only: cask.discontinued?)
|
||||||
end
|
end
|
||||||
|
|
||||||
sig { void }
|
sig { void }
|
||||||
|
|||||||
@ -70,7 +70,7 @@ module Cask
|
|||||||
errors += audit.errors
|
errors += audit.errors
|
||||||
end
|
end
|
||||||
|
|
||||||
{ errors: errors }
|
errors
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|||||||
@ -60,7 +60,7 @@ module Cask
|
|||||||
except: [],
|
except: [],
|
||||||
)
|
)
|
||||||
|
|
||||||
failed_casks = results.reject { |_, result| result[:errors].empty? }.map(&:first)
|
failed_casks = results.reject { |_, result| result.empty? }.map(&:first)
|
||||||
return if failed_casks.empty?
|
return if failed_casks.empty?
|
||||||
|
|
||||||
raise CaskError, "audit failed for casks: #{failed_casks.join(" ")}"
|
raise CaskError, "audit failed for casks: #{failed_casks.join(" ")}"
|
||||||
|
|||||||
@ -266,11 +266,11 @@ module Homebrew
|
|||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
failed_casks = cask_results.reject { |_, result| result[:errors].empty? }
|
failed_casks = cask_results.reject { |_, result| result.empty? }
|
||||||
|
|
||||||
cask_count = failed_casks.count
|
cask_count = failed_casks.count
|
||||||
|
|
||||||
cask_problem_count = failed_casks.sum { |_, result| result[:warnings].count + result[:errors].count }
|
cask_problem_count = failed_casks.sum { |_, result| result.count }
|
||||||
new_formula_problem_count += new_formula_problem_lines.count
|
new_formula_problem_count += new_formula_problem_lines.count
|
||||||
total_problems_count = problem_count + new_formula_problem_count + cask_problem_count + tap_problem_count
|
total_problems_count = problem_count + new_formula_problem_count + cask_problem_count + tap_problem_count
|
||||||
|
|
||||||
|
|||||||
@ -101,7 +101,7 @@ describe Cask::Audit, :cask do
|
|||||||
|
|
||||||
context "when there are no errors and `--strict` is not passed so we should not show anything" do
|
context "when there are no errors and `--strict` is not passed so we should not show anything" do
|
||||||
before do
|
before do
|
||||||
audit.add_error("eh", strictish: true)
|
audit.add_error("eh", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.not_to match(/failed/) }
|
it { is_expected.not_to match(/failed/) }
|
||||||
@ -118,7 +118,7 @@ describe Cask::Audit, :cask do
|
|||||||
context "when there are errors and warnings" do
|
context "when there are errors and warnings" do
|
||||||
before do
|
before do
|
||||||
audit.add_error "bad"
|
audit.add_error "bad"
|
||||||
audit.add_error("eh", strictish: true)
|
audit.add_error("eh", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.to match(/failed/) }
|
it { is_expected.to match(/failed/) }
|
||||||
@ -129,7 +129,7 @@ describe Cask::Audit, :cask do
|
|||||||
|
|
||||||
before do
|
before do
|
||||||
audit.add_error "very bad"
|
audit.add_error "very bad"
|
||||||
audit.add_error("a little bit bad", strictish: true)
|
audit.add_error("a little bit bad", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.to match(/failed/) }
|
it { is_expected.to match(/failed/) }
|
||||||
@ -137,7 +137,7 @@ describe Cask::Audit, :cask do
|
|||||||
|
|
||||||
context "when there are warnings and `--strict` is not passed" do
|
context "when there are warnings and `--strict` is not passed" do
|
||||||
before do
|
before do
|
||||||
audit.add_error("a little bit bad", strictish: true)
|
audit.add_error("a little bit bad", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.not_to match(/failed/) }
|
it { is_expected.not_to match(/failed/) }
|
||||||
@ -147,7 +147,7 @@ describe Cask::Audit, :cask do
|
|||||||
let(:strict) { true }
|
let(:strict) { true }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
audit.add_error("a little bit bad", strictish: true)
|
audit.add_error("a little bit bad", strict_only: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it { is_expected.to match(/failed/) }
|
it { is_expected.to match(/failed/) }
|
||||||
|
|||||||
@ -6,7 +6,7 @@ require "cask/auditor"
|
|||||||
describe Cask::Cmd::Audit, :cask do
|
describe Cask::Cmd::Audit, :cask do
|
||||||
let(:cask) { Cask::Cask.new("cask") }
|
let(:cask) { Cask::Cask.new("cask") }
|
||||||
let(:cask_with_many_languages) { Cask::CaskLoader.load(cask_path("with-many-languages")) }
|
let(:cask_with_many_languages) { Cask::CaskLoader.load(cask_path("with-many-languages")) }
|
||||||
let(:result) { { warnings: Set.new, errors: Set.new } }
|
let(:result) { Set.new }
|
||||||
|
|
||||||
describe "selection of Casks to audit" do
|
describe "selection of Casks to audit" do
|
||||||
it "audits all Casks if no tokens are given" do
|
it "audits all Casks if no tokens are given" do
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user