Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7c3cdd2
Add rubocop-performance as development dependency
mattt Aug 25, 2022
1fcd202
Enable Performance department cops
mattt Aug 25, 2022
c46db82
Fix Performance/RedundantEqualityComparisonBlock offenses
mattt Sep 2, 2022
7d41a04
Fix Performance/MapCompact offenses
mattt Sep 2, 2022
c60cca6
Fix Performance/Count offenses
mattt Sep 2, 2022
55d68bb
Fix Performance/ReverseFirst offenses
mattt Sep 2, 2022
3e363c0
Fix Performance/Detect offenses
mattt Sep 2, 2022
cb1a57b
Fix Performance/DoubleStartEndWith offenses
mattt Sep 2, 2022
dcacf4a
Fix Performance/StringReplacement offenses
mattt Sep 2, 2022
3459223
Fix Performance/StringInclude offenses
mattt Sep 2, 2022
cdec8d7
Fix Performance/DeletePrefix offenses
mattt Sep 2, 2022
1ec095d
Fix Performance/Casecmp offenses
mattt Sep 2, 2022
03f45c7
Fix Performance/Squeeze offenses
mattt Sep 2, 2022
23fc98f
Fix Performance/UnfreezeString offenses
mattt Sep 2, 2022
0615cd9
Fix Performance/RegexpMatch offenses
mattt Sep 2, 2022
1a1e440
Fix Style/StringLiterals offenses
mattt Sep 2, 2022
18bdc8a
Fix Layout/MultilineMethodCallIndentation offenses
mattt Sep 2, 2022
09762af
Fix Layout/DotPosition offenses
mattt Sep 2, 2022
f6a0bd0
Fix Layout/TrailingWhitespace offenses
mattt Sep 2, 2022
18f5d3c
Fix Performance/ConstantRegexp offenses
mattt Sep 2, 2022
55e7561
Fix Performance/CaseWhenSplat offenses
mattt Sep 2, 2022
df73a78
Fix Performance/ReverseEach offenses
mattt Sep 2, 2022
3c78584
Fix remaining Style, Layout, and Lint offenses
mattt Sep 2, 2022
52f09c8
Fix Performance/MethodObjectAsBlock offenses
mattt Sep 2, 2022
afc4f3a
Fix Performance/Count offense
mattt Sep 2, 2022
6bec4c3
Fix Performance/CollectionLiteralInLoop offenses
mattt Sep 2, 2022
d00b8aa
Fix Performance/RedundantBlockCall offenses
mattt Sep 2, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
---
require: rubocop-performance

AllCops:
DisplayCopNames: true
Exclude:
Expand Down Expand Up @@ -122,6 +124,102 @@ Metrics/PerceivedComplexity:
Max: 10
Naming/FileName:
Enabled: false
Performance/AncestorsInclude:
Enabled: true
Performance/BigDecimalWithNumericArgument:
Enabled: true
Performance/BindCall:
Enabled: true
Performance/BlockGivenWithExplicitBlock:
Enabled: true
Performance/Caller:
Enabled: true
Performance/CaseWhenSplat:
Enabled: true
Performance/Casecmp:
Enabled: true
Performance/ChainArrayAllocation:
Enabled: false
Performance/CollectionLiteralInLoop:
Enabled: true
Performance/CompareWithBlock:
Enabled: true
Performance/ConcurrentMonotonicTime:
Enabled: true
Performance/ConstantRegexp:
Enabled: true
Performance/Count:
Enabled: true
Performance/DeletePrefix:
Enabled: true
Performance/DeleteSuffix:
Enabled: true
Performance/Detect:
Enabled: true
Performance/DoubleStartEndWith:
Enabled: true
Performance/EndWith:
Enabled: true
Performance/FixedSize:
Enabled: true
Performance/FlatMap:
Enabled: true
Performance/InefficientHashSearch:
Enabled: true
Performance/IoReadlines:
Enabled: true
Performance/MapCompact:
Enabled: true
Performance/MethodObjectAsBlock:
Enabled: true
Performance/OpenStruct:
Enabled: false
Performance/RangeInclude:
Enabled: true
Performance/RedundantBlockCall:
Enabled: true
Performance/RedundantEqualityComparisonBlock:
Enabled: true
Performance/RedundantMatch:
Enabled: true
Performance/RedundantMerge:
Enabled: true
Performance/RedundantSortBlock:
Enabled: true
Performance/RedundantSplitRegexpArgument:
Enabled: true
Performance/RedundantStringChars:
Enabled: true
Performance/RegexpMatch:
Enabled: true
Performance/ReverseEach:
Enabled: true
Performance/ReverseFirst:
Enabled: true
Performance/SelectMap:
Enabled: false
Performance/Size:
Enabled: true
Performance/SortReverse:
Enabled: true
Performance/Squeeze:
Enabled: true
Performance/StartWith:
Enabled: true
Performance/StringIdentifierArgument:
Enabled: true
Performance/StringInclude:
Enabled: true
Performance/StringReplacement:
Enabled: true
Performance/Sum:
Enabled: true
Performance/TimesMap:
Enabled: true
Performance/UnfreezeString:
Enabled: true
Performance/UriDefaultParser:
Enabled: true
Style/AccessorGrouping:
Enabled: false
Style/ArgumentsForwarding:
Expand Down Expand Up @@ -170,6 +268,8 @@ Style/KeywordParametersOrder:
Enabled: false
Style/MultilineInPatternThen:
Enabled: true
Style/MultipleComparison:
Enabled: false
Style/NegatedIfElseCondition:
Enabled: true
Style/NilLambda:
Expand Down
14 changes: 8 additions & 6 deletions bundler/helpers/v1/lib/functions/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ def parsed_gemfile(gemfile_name:)
Bundler::Definition.build(gemfile_name, nil, {}).
dependencies.select(&:current_platform?).
reject { |dep| dep.source.is_a?(Bundler::Source::Gemspec) }.
map(&method(:serialize_bundler_dependency))
map { |dep| serialize_bundler_dependency(dep) }
end

def parsed_gemspec(gemspec_name:)
Bundler.load_gemspec_uncached(gemspec_name).
dependencies.
map(&method(:serialize_bundler_dependency))
map { |dep| serialize_bundler_dependency(dep) }
end

private
Expand Down 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
4 changes: 2 additions & 2 deletions bundler/helpers/v1/lib/functions/lockfile_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ def unlock_blocking_subdeps(dependencies_to_unlock, error)
potentials_deps =
error.cause.conflicts.values.
flat_map(&:requirement_trees).
map do |tree|
filter_map do |tree|
tree.find { |req| allowed_new_unlocks.include?(req.name) }
end.compact.map(&:name)
end.map(&:name)

# If there are specific dependencies we can unlock, unlock them
return dependencies_to_unlock.append(*potentials_deps) if potentials_deps.any?
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
14 changes: 8 additions & 6 deletions bundler/helpers/v2/lib/functions/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ def parsed_gemfile(gemfile_name:)
Bundler::Definition.build(gemfile_name, nil, {}).
dependencies.select(&:current_platform?).
reject { |dep| dep.source.is_a?(Bundler::Source::Gemspec) }.
map(&method(:serialize_bundler_dependency))
map { |dep| serialize_bundler_dependency(dep) }
end

def parsed_gemspec(gemspec_name:)
Bundler.load_gemspec_uncached(gemspec_name).
dependencies.
map(&method(:serialize_bundler_dependency))
map { |dep| serialize_bundler_dependency(dep) }
end

private
Expand Down 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
4 changes: 2 additions & 2 deletions bundler/helpers/v2/lib/functions/lockfile_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ def unlock_blocking_subdeps(dependencies_to_unlock, error)
potentials_deps =
error.cause.conflicts.values.
flat_map(&:requirement_trees).
map do |tree|
filter_map do |tree|
tree.find { |req| allowed_new_unlocks.include?(req.name) }
end.compact.map(&:name)
end.map(&:name)

# If there are specific dependencies we can unlock, unlock them
return dependencies_to_unlock.append(*potentials_deps) if potentials_deps.any?
Expand Down
2 changes: 1 addition & 1 deletion bundler/helpers/v2/monkey_patches/git_source_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class GitProxy
# Instead, we convert all `git@github.com:` URLs to use HTTPS.
def configured_uri_for(uri)
uri = uri.gsub(%r{git@(.*?):/?}, 'https://\1/')
if /https?:/ =~ uri
if /https?:/.match?(uri)
remote = Bundler::URI(uri)
config_auth = Bundler.settings[remote.to_s] || Bundler.settings[remote.host]
remote.userinfo ||= config_auth
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
4 changes: 2 additions & 2 deletions bundler/lib/dependabot/bundler/metadata_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def find_source_from_rubygems_api_response
end

def find_source_from_git_url
info = dependency.requirements.map { |r| r[:source] }.compact.first
info = dependency.requirements.filter_map { |r| r[:source] }.first

url = info[:url] || info.fetch("url")
Source.from_url(url)
Expand Down Expand Up @@ -198,7 +198,7 @@ def augment_private_response_if_appropriate(response_body)
def registry_url
return "https://rubygems.org/" if new_source_type == "default"

info = dependency.requirements.map { |r| r[:source] }.compact.first
info = dependency.requirements.filter_map { |r| r[:source] }.first
info[:url] || info.fetch("url")
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ def dependencies_from(updated_deps, specs)
#
# This is kind of a bug in Bundler, and we should try to fix it,
# but resolving it won't necessarily be easy.
updated_deps.map do |dep|
updated_deps.filter_map do |dep|
original_dep =
original_dependencies.find { |d| d.name == dep.fetch("name") }
spec = specs.find { |d| d.fetch("name") == dep.fetch("name") }

next if spec.fetch("version") == original_dep.version

build_dependency(original_dep, spec)
end.compact
end
end

def build_dependency(original_dep, updated_spec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def initialize(requirements:, update_strategy:, updated_source:,

def updated_requirements
requirements.map do |req|
if req[:file].match?(/\.gemspec/)
if req[:file].include?(".gemspec")
update_gemspec_requirement(req)
else
# If a requirement doesn't come from a gemspec, it must be from
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
6 changes: 3 additions & 3 deletions bundler/spec/dependabot/bundler/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -555,11 +555,11 @@
let(:dependency_files) { bundler_project_dependency_files("imports_gemspec_imports_gemspec_large") }

it "includes details of each declaration" do
expect(dependencies.select(&:top_level?).count).to eq(13)
expect(dependencies.count(&:top_level?)).to eq(13)
end

it "includes details of each sub-dependency" do
expect(dependencies.reject(&:top_level?).count).to eq(23)
expect(dependencies.count { |dep| !dep.top_level? }).to eq(23)

diff_lcs = dependencies.find { |d| d.name == "diff-lcs" }
expect(diff_lcs.subdependency_metadata).to eq([{ production: false }])
Expand Down Expand Up @@ -607,7 +607,7 @@
let(:dependency_files) { bundler_project_dependency_files("imports_gemspec_with_require") }

it "includes details of each declaration" do
expect(dependencies.select(&:top_level?).count).to eq(13)
expect(dependencies.count(&:top_level?)).to eq(13)
end
end

Expand Down
5 changes: 2 additions & 3 deletions cargo/lib/dependabot/cargo/metadata_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ def new_source_type
def find_source_from_crates_listing
potential_source_urls =
SOURCE_KEYS.
map { |key| crates_listing.dig("crate", key) }.
compact
filter_map { |key| crates_listing.dig("crate", key) }

source_url = potential_source_urls.find { |url| Source.from_url(url) }
Source.from_url(source_url)
end

def find_source_from_git_url
info = dependency.requirements.map { |r| r[:source] }.compact.first
info = dependency.requirements.filter_map { |r| r[:source] }.first

url = info[:url] || info.fetch("url")
Source.from_url(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,7 @@ def lower_bound_version
dependency.version
else
version_from_requirement =
dependency.requirements.map { |r| r.fetch(:requirement) }.
compact.
dependency.requirements.filter_map { |r| r.fetch(:requirement) }.
flat_map { |req_str| Cargo::Requirement.new(req_str) }.
flat_map(&:requirements).
reject { |req_array| req_array.first.start_with?("<") }.
Expand Down
Loading