Skip to content

Commit

Permalink
Fix Performance/CollectionLiteralInLoop offenses
Browse files Browse the repository at this point in the history
  • Loading branch information
mattt committed Sep 2, 2022
1 parent df3adef commit 7fc81d0
Show file tree
Hide file tree
Showing 17 changed files with 41 additions and 29 deletions.
10 changes: 6 additions & 4 deletions bundler/helpers/v1/lib/functions/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,17 @@ def git_source_details(source)
}
end

RUBYGEMS_HOSTS = [
"rubygems.org",
"www.rubygems.org"
].freeze

def default_rubygems?(source)
return true if source.nil?
return false unless source.is_a?(Bundler::Source::Rubygems)

source.remotes.any? do |r|
[
"rubygems.org",
"www.rubygems.org"
].include?(URI(r.to_s).host)
RUBYGEMS_HOSTS.include?(URI(r.to_s).host)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def entries
opts = {}
opts[:encoding] = ::Encoding::UTF_8 if fu_windows?
Dir.entries(path, **opts).
reject { |n| [".", ".."].include?(n) }.
reject { |n| n == "." || n == ".." }.
map { |n| self.class.new(prefix, join(rel, n.untaint)) }
end
end
Expand Down
10 changes: 6 additions & 4 deletions bundler/helpers/v2/lib/functions/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,17 @@ def git_source_details(source)
}
end

RUBYGEMS_HOSTS = [
"rubygems.org",
"www.rubygems.org"
].freeze

def default_rubygems?(source)
return true if source.nil?
return false unless source.is_a?(Bundler::Source::Rubygems)

source.remotes.any? do |r|
[
"rubygems.org",
"www.rubygems.org"
].include?(URI(r.to_s).host)
RUBYGEMS_HOSTS.include?(URI(r.to_s).host)
end
end

Expand Down
2 changes: 1 addition & 1 deletion bundler/helpers/v2/spec/functions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def expect_specs(count)
expect(git_specs.size).to eq(count)
git_specs.each do |gs|
uri = URI.parse(gs[:auth_uri])
expect(uri.scheme).to(satisfy { |s| %w(http https).include?(s) })
expect(uri.scheme).to(satisfy { |s| s.match?(/https?/o) })
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ module Dependabot
module Bundler
class FileUpdater
class GemfileUpdater
GEMFILE_FILENAMES = %w(Gemfile gems.rb).freeze

require_relative "git_pin_replacer"
require_relative "git_source_remover"
require_relative "requirement_replacer"
Expand Down Expand Up @@ -68,21 +70,21 @@ def requirement_changed?(file, dependency)
def remove_git_source?(dependency)
old_gemfile_req =
dependency.previous_requirements.
find { |f| %w(Gemfile gems.rb).include?(f[:file]) }
find { |f| GEMFILE_FILENAMES.include?(f[:file]) }

return false unless old_gemfile_req&.dig(:source, :type) == "git"

new_gemfile_req =
dependency.requirements.
find { |f| %w(Gemfile gems.rb).include?(f[:file]) }
find { |f| GEMFILE_FILENAMES.include?(f[:file]) }

new_gemfile_req[:source].nil?
end

def update_git_pin?(dependency)
new_gemfile_req =
dependency.requirements.
find { |f| %w(Gemfile gems.rb).include?(f[:file]) }
find { |f| GEMFILE_FILENAMES.include?(f[:file]) }
return false unless new_gemfile_req&.dig(:source, :type) == "git"

# If the new requirement is a git dependency with a ref then there's
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def inaccessible_git_dependencies
)
git_specs.reject do |spec|
uri = URI.parse(spec.fetch("auth_uri"))
next false unless %w(http https).include?(uri.scheme)
next false unless uri.scheme.match?(/https?/o)

Dependabot::RegistryClient.get(
url: uri.to_s
Expand Down
2 changes: 1 addition & 1 deletion common/lib/dependabot/file_fetchers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def _cloned_repo_contents(relative_path)
return [] unless Dir.exist?(repo_path)

Dir.entries(repo_path).filter_map do |name|
next if [".", ".."].include?(name)
next if name == "." || name == ".."

absolute_path = File.join(repo_path, name)
type = if File.symlink?(absolute_path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def fetch_github_file_list(ref)
files += github_client.contents(source.repo, opts)

files.uniq.each do |f|
next unless %w(doc docs).include?(f.name) && f.type == "dir"
next unless f.type == "dir" && f.name.match?(/docs?/o)

opts = { path: f.path, ref: ref }.compact
files += github_client.contents(source.repo, opts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,8 @@ def updated_elm_json_content(content)
# `elm install <dependency_name>` to generate the install plan
%w(dependencies test-dependencies).each do |type|
json[type].delete(dependency.name) if json.dig(type, dependency.name)

%w(direct indirect).each do |category|
json[type][category].delete(dependency.name) if json.dig(type, category, dependency.name)
end
json[type]["direct"].delete(dependency.name) if json.dig(type, "direct", dependency.name)
json[type]["indirect"].delete(dependency.name) if json.dig(type, "indirect", dependency.name)
end

json["source-directories"] = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def repository_urls
end

def check_response(response, repository_url)
return unless [401, 403].include?(response.status)
return unless response.status == 401 || response.status == 403
return if @forbidden_urls.include?(repository_url)
return if central_repo_urls.include?(repository_url)

Expand Down
2 changes: 1 addition & 1 deletion gradle/spec/dependabot/gradle/file_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
describe "the updated build.gradle file" do
subject(:updated_buildfile) do
updated_files.find do |f|
%w(build.gradle build.gradle.kts).include?(f.name)
Dependabot::Gradle::FileUpdater::SUPPORTED_BUILD_FILE_NAMES.include?(f.name)
end
end

Expand Down
3 changes: 2 additions & 1 deletion python/lib/dependabot/python/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module Python
class FileFetcher < Dependabot::FileFetchers::Base
CHILD_REQUIREMENT_REGEX = /^-r\s?(?<path>.*\.(?:txt|in))/.freeze
CONSTRAINT_REGEX = /^-c\s?(?<path>.*\.(?:txt|in))/.freeze
DEPENDENCY_TYPES = %w(packages dev-packages).freeze

def self.required_files_in?(filenames)
return true if filenames.any? { |name| name.end_with?(".txt", ".in") }
Expand Down Expand Up @@ -372,7 +373,7 @@ def pipfile_path_setup_file_paths
return [] unless pipfile

paths = []
%w(packages dev-packages).each do |dep_type|
DEPENDENCY_TYPES.each do |dep_type|
next unless parsed_pipfile[dep_type]

parsed_pipfile[dep_type].each do |_, req|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ def parse_requirements_from(req, type)
def lockfile_dependencies
dependencies = Dependabot::FileParsers::Base::DependencySet.new

source_types = %w(directory git url)
parsed_lockfile.fetch("package", []).each do |details|
next if %w(directory git url).include?(details.dig("source", "type"))
next if source_types.include?(details.dig("source", "type"))

dependencies <<
Dependency.new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class PipfileFileUpdater
require_relative "pipfile_manifest_updater"
require_relative "setup_file_sanitizer"

DEPENDENCY_TYPES = %w(packages dev-packages).freeze

attr_reader :dependencies, :dependency_files, :credentials

def initialize(dependencies:, dependency_files:, credentials:)
Expand Down Expand Up @@ -145,7 +147,7 @@ def freeze_dependencies_being_updated(pipfile_content)
pipfile_object = TomlRB.parse(pipfile_content)

dependencies.each do |dep|
%w(packages dev-packages).each do |type|
DEPENDENCY_TYPES.each do |type|
names = pipfile_object[type]&.keys || []
pkg_name = names.find { |nm| normalise(nm) == dep.name }
next unless pkg_name || subdep_type?(type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,15 @@ def freeze_top_level_dependencies_except(dependencies)
Dependabot::Python::FileParser::PoetryFilesParser::POETRY_DEPENDENCY_TYPES.each do |key|
next unless poetry_object[key]

source_types = %w(directory file url)
poetry_object.fetch(key).each do |dep_name, _|
next if excluded_names.include?(normalise(dep_name))

locked_details = locked_details(dep_name)

next unless (locked_version = locked_details&.fetch("version"))

next if %w(directory file url).include?(locked_details&.dig("source", "type"))
next if source_types.include?(locked_details&.dig("source", "type"))

if locked_details&.dig("source", "type") == "git"
poetry_object[key][dep_name] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,10 @@ def available_versions
index_urls.flat_map do |index_url|
sanitized_url = index_url.gsub(%r{(?<=//).*(?=@)}, "redacted")
index_response = registry_response_for_dependency(index_url)
registry_index_response = registry_index_response(index_url)

if [401, 403].include?(index_response.status) &&
[401, 403].include?(registry_index_response(index_url).status)
if (index_response.status == 401 || index_response.status == 403) &&
(registry_index_response.status == 401 || registry_index_response.status == 403)
raise PrivateSourceAuthenticationFailure, sanitized_url
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class PipenvVersionResolver
PIPENV_RANGE_WARNING = /Warning:\sPython\s[<>].* was not found/.freeze
# rubocop:enable Layout/LineLength

DEPENDENCY_TYPES = %w(packages dev-packages).freeze

attr_reader :dependency, :dependency_files, :credentials

def initialize(dependency:, dependency_files:, credentials:)
Expand Down Expand Up @@ -363,7 +365,7 @@ def set_target_dependency_req(pipfile_content, updated_requirement)

pipfile_object = TomlRB.parse(pipfile_content)

%w(packages dev-packages).each do |type|
DEPENDENCY_TYPES.each do |type|
names = pipfile_object[type]&.keys || []
pkg_name = names.find { |nm| normalise(nm) == dependency.name }
next unless pkg_name || subdep_type?(type)
Expand Down

0 comments on commit 7fc81d0

Please sign in to comment.