Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added sorbet typecheck under python module on 5 files #11261

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b62d316
Added sorbet typecheck under python module for setup_file_parser.rb ,…
randhircs Jan 9, 2025
4047bcb
Fixed issue in index_finder
randhircs Jan 9, 2025
0c7d855
Fixed issue in setupfilesanitizer
randhircs Jan 9, 2025
0682f60
Included TypeCheck for one more file
randhircs Jan 9, 2025
c6b8aca
Remvoed from index finder file
randhircs Jan 9, 2025
a0527c8
Removed from pyproject_preparer
randhircs Jan 10, 2025
35e77ed
Commented since return type is not correct
randhircs Jan 10, 2025
036aa7f
Added TypeCheck back to setup_file_parser
randhircs Jan 10, 2025
c8aa9af
Added TypeCheck pyproject_preparer.rb
randhircs Jan 10, 2025
e1b473c
Removed commented code
randhircs Jan 10, 2025
a7fae08
Added TypeCheck as strict for file_parser
randhircs Jan 13, 2025
b32c50c
Added TypeCheck as strict for file_parser
randhircs Jan 13, 2025
9586bea
Removed nilable to fix the issue
randhircs Jan 14, 2025
611444e
Moved back to untyped
randhircs Jan 14, 2025
291f9ab
Moved back to untyped
randhircs Jan 14, 2025
34a78c7
Moved back to untyped
randhircs Jan 14, 2025
955f98a
corrected return type
randhircs Jan 14, 2025
a58b3e5
Corrected returntype to pass end to end error
randhircs Jan 14, 2025
8442af3
Changed requirementfile method return type
randhircs Jan 14, 2025
e6d6fac
Changed return type to dependabotset
randhircs Jan 14, 2025
e5261c5
Changed return type to dependencySet
randhircs Jan 14, 2025
d9dfa0f
Corrected log_if_version_malformed
randhircs Jan 14, 2025
f44145f
Changed return type in pipenv_runner
randhircs Jan 14, 2025
7dea911
Corrected return type
randhircs Jan 15, 2025
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
111 changes: 69 additions & 42 deletions python/lib/dependabot/python/file_parser.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# typed: true
# typed: strict
# frozen_string_literal: true

require "dependabot/dependency"
Expand All @@ -17,13 +17,14 @@

module Dependabot
module Python
class FileParser < Dependabot::FileParsers::Base
class FileParser < Dependabot::FileParsers::Base # rubocop:disable Metrics/ClassLength
extend T::Sig
require_relative "file_parser/pipfile_files_parser"
require_relative "file_parser/pyproject_files_parser"
require_relative "file_parser/setup_file_parser"
require_relative "file_parser/python_requirement_parser"

DEPENDENCY_GROUP_KEYS = [
DEPENDENCY_GROUP_KEYS = T.let([
{
pipfile: "packages",
lockfile: "default"
Expand All @@ -32,7 +33,7 @@ class FileParser < Dependabot::FileParsers::Base
pipfile: "dev-packages",
lockfile: "develop"
}
].freeze
].freeze, T::Array[T::Hash[Symbol, String]])
REQUIREMENT_FILE_EVALUATION_ERRORS = %w(
InstallationError RequirementsFileParseError InvalidMarker
InvalidRequirement ValueError RecursionError
Expand All @@ -43,6 +44,7 @@ class FileParser < Dependabot::FileParsers::Base
# in any way if any metric collection exception start happening
UNDETECTED_PACKAGE_MANAGER_VERSION = "0.0"

sig { override.returns(T::Array[Dependabot::Dependency]) }
def parse
# TODO: setup.py from external dependencies is evaluated. Provide guards before removing this.
raise Dependabot::UnexpectedExternalCode if @reject_external_code
Expand All @@ -57,7 +59,7 @@ def parse
dependency_set.dependencies
end

sig { returns(Ecosystem) }
sig { override.returns(Ecosystem) }
def ecosystem
@ecosystem ||= T.let(
Ecosystem.new(
Expand All @@ -71,18 +73,16 @@ def ecosystem

private

sig { returns(Dependabot::Python::LanguageVersionManager) }
def language_version_manager
@language_version_manager ||=
LanguageVersionManager.new(
python_requirement_parser: python_requirement_parser
)
@language_version_manager ||= T.let(LanguageVersionManager.new(python_requirement_parser:
python_requirement_parser), T.nilable(LanguageVersionManager))
end

sig { returns(Dependabot::Python::FileParser::PythonRequirementParser) }
def python_requirement_parser
@python_requirement_parser ||=
FileParser::PythonRequirementParser.new(
dependency_files: dependency_files
)
@python_requirement_parser ||= T.let(FileParser::PythonRequirementParser.new(dependency_files:
dependency_files), T.nilable(FileParser::PythonRequirementParser))
end

sig { returns(Ecosystem::VersionManager) }
Expand All @@ -91,7 +91,7 @@ def package_manager
Dependabot.logger.info("Detected package manager : #{detected_package_manager.name}")
end

@package_manager ||= detected_package_manager
@package_manager ||= T.let(detected_package_manager, T.nilable(Dependabot::Ecosystem::VersionManager))
end

sig { returns(Ecosystem::VersionManager) }
Expand Down Expand Up @@ -188,7 +188,7 @@ def package_manager_version(package_manager)
end

# setup python local setup on file parser stage
sig { void }
sig { returns(T.nilable(String)) }
def setup_python_environment
language_version_manager.install_required_python

Expand All @@ -198,14 +198,15 @@ def setup_python_environment
nil
end

sig { params(package_manager: String, version: String).void }
sig { params(package_manager: String, version: String).returns(T::Boolean) }
def log_if_version_malformed(package_manager, version)
# logs warning if malformed version is found
return true if version.match?(/^\d+(?:\.\d+)*$/)

Dependabot.logger.warn(
"Detected #{package_manager} with malformed version #{version}"
)
if version.match?(/^\d+(?:\.\d+)*$/)
true
else
Dependabot.logger.warn("Detected #{package_manager} with malformed version #{version}")
false
end
end

sig { returns(String) }
Expand All @@ -231,24 +232,24 @@ def language
)
end

sig { returns(T::Array[Dependabot::DependencyFile]) }
def requirement_files
dependency_files.select { |f| f.name.end_with?(".txt", ".in") }
end

sig { returns(DependencySet) }
def pipenv_dependencies
@pipenv_dependencies ||=
PipfileFilesParser
.new(dependency_files: dependency_files)
.dependency_set
@pipenv_dependencies ||= T.let(PipfileFilesParser.new(dependency_files:
dependency_files).dependency_set, T.nilable(DependencySet))
end

sig { returns(DependencySet) }
def pyproject_file_dependencies
@pyproject_file_dependencies ||=
PyprojectFilesParser
.new(dependency_files: dependency_files)
.dependency_set
@pyproject_file_dependencies ||= T.let(PyprojectFilesParser.new(dependency_files:
dependency_files).dependency_set, T.nilable(DependencySet))
end

sig { returns(DependencySet) }
def requirement_dependencies
dependencies = DependencySet.new
parsed_requirement_files.each do |dep|
Expand Down Expand Up @@ -286,20 +287,23 @@ def requirement_dependencies
dependencies
end

sig { params(name: T.nilable(String), version: T.nilable(String)).returns(T::Boolean) }
def old_pyyaml?(name, version)
major_version = version&.split(".")&.first
return false unless major_version

name == "pyyaml" && major_version < "6"
end

sig { params(filename: String).returns(T::Array[String]) }
def group_from_filename(filename)
if filename.include?("dev") then ["dev-dependencies"]
else
["dependencies"]
end
end

sig { params(dep: T.untyped).returns(T::Boolean) }
def blocking_marker?(dep)
return false if dep["markers"] == "None"
Copy link
Contributor

@kbukum1 kbukum1 Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this T.untyped? Is it something changing dependency to the situation?


Expand All @@ -316,6 +320,9 @@ def blocking_marker?(dep)
end
end

sig do
params(marker: T.untyped, python_version: T.any(String, Integer, Gem::Version)).returns(T::Boolean)
end
def marker_satisfied?(marker, python_version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think T.untyped is not proper unless necessary

conditions = marker.split(/\s+(and|or)\s+/)

Expand All @@ -337,6 +344,10 @@ def marker_satisfied?(marker, python_version)
result
end

sig do
params(condition: T.untyped,
python_version: T.any(String, Integer, Gem::Version)).returns(T::Boolean)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, T.untyped?

end
def evaluate_condition(condition, python_version)
operator, version = condition.match(/([<>=!]=?)\s*"?([\d.]+)"?/)&.captures

Expand All @@ -356,13 +367,13 @@ def evaluate_condition(condition, python_version)
end
end

sig { returns(DependencySet) }
def setup_file_dependencies
@setup_file_dependencies ||=
SetupFileParser
.new(dependency_files: dependency_files)
.dependency_set
@setup_file_dependencies ||= T.let(SetupFileParser.new(dependency_files: dependency_files)
.dependency_set, T.nilable(DependencySet))
end

sig { returns(T.untyped) }
def parsed_requirement_files
Copy link
Contributor

@kbukum1 kbukum1 Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do have return type here but not sure.

SharedHelpers.in_a_temporary_directory do
write_temporary_dependency_files
Expand All @@ -383,6 +394,7 @@ def parsed_requirement_files
raise Dependabot::DependencyFileNotEvaluatable, e.message
end

sig { params(requirements: T.untyped).returns(T.untyped) }
def check_requirements(requirements)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are requirements actually untyped?

requirements.each do |dep|
next unless dep["requirement"]
Expand All @@ -393,18 +405,22 @@ def check_requirements(requirements)
end
end

sig { returns(T::Boolean) }
def pipcompile_in_file
requirement_files.any? { |f| f.name.end_with?(PipCompilePackageManager::MANIFEST_FILENAME) }
end

sig { returns(T::Boolean) }
def pipenv_files
dependency_files.any? { |f| f.name == PipenvPackageManager::LOCKFILE_FILENAME }
end

sig { returns(T.nilable(TrueClass)) }
def poetry_files
true if get_original_file(PoetryPackageManager::LOCKFILE_NAME)
end

sig { returns(T::Array[Dependabot::DependencyFile]) }
def write_temporary_dependency_files
dependency_files
.reject { |f| f.name == ".python-version" }
Expand All @@ -415,6 +431,7 @@ def write_temporary_dependency_files
end
end

sig { params(file: T.untyped).returns(T.untyped) }
def remove_imports(file)
return file.content if file.path.end_with?(".tar.gz", ".whl", ".zip")

Expand All @@ -424,10 +441,12 @@ def remove_imports(file)
.join
end

sig { params(name: String, extras: T::Array[String]).returns(String) }
def normalised_name(name, extras = [])
NameNormaliser.normalise_including_extras(name, extras)
end

sig { override.returns(T.untyped) }
def check_required_files
filenames = dependency_files.map(&:name)
return if filenames.any? { |name| name.end_with?(".txt", ".in") }
Expand All @@ -439,37 +458,45 @@ def check_required_files
raise "Missing required files!"
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def pipfile
@pipfile ||= get_original_file("Pipfile")
@pipfile ||= T.let(get_original_file("Pipfile"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def pipfile_lock
@pipfile_lock ||= get_original_file("Pipfile.lock")
@pipfile_lock ||= T.let(get_original_file("Pipfile.lock"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def pyproject
@pyproject ||= get_original_file("pyproject.toml")
@pyproject ||= T.let(get_original_file("pyproject.toml"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def poetry_lock
@poetry_lock ||= get_original_file("poetry.lock")
@poetry_lock ||= T.let(get_original_file("poetry.lock"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def setup_file
@setup_file ||= get_original_file("setup.py")
@setup_file ||= T.let(get_original_file("setup.py"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T.nilable(Dependabot::DependencyFile)) }
def setup_cfg_file
@setup_cfg_file ||= get_original_file("setup.cfg")
@setup_cfg_file ||= T.let(get_original_file("setup.cfg"), T.nilable(Dependabot::DependencyFile))
end

sig { returns(T::Array[Dependabot::Python::Requirement]) }
def pip_compile_files
@pip_compile_files ||=
dependency_files.select { |f| f.name.end_with?(".in") }
@pip_compile_files ||= T.let(dependency_files.select { |f| f.name.end_with?(".in") }, T.untyped)
end

sig { returns(Dependabot::Python::PipCompileFileMatcher) }
def pip_compile_file_matcher
@pip_compile_file_matcher ||= PipCompileFileMatcher.new(pip_compile_files)
@pip_compile_file_matcher ||= T.let(PipCompileFileMatcher.new(pip_compile_files),
T.nilable(Dependabot::Python::PipCompileFileMatcher))
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
require "dependabot/python/file_parser"
require "dependabot/python/native_helpers"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update the typed: strict here too?

require "dependabot/python/name_normaliser"
require "sorbet-runtime"

module Dependabot
module Python
class FileParser
class SetupFileParser
extend T::Sig
INSTALL_REQUIRES_REGEX = /install_requires\s*=\s*\[/m
SETUP_REQUIRES_REGEX = /setup_requires\s*=\s*\[/m
TESTS_REQUIRE_REGEX = /tests_require\s*=\s*\[/m
Expand Down Expand Up @@ -141,18 +143,21 @@ def write_sanitized_setup_file
File.write("setup.py", tmp)
end

sig { params(regex: Regexp).returns(T.nilable(String)) }
def get_regexed_req_array(regex)
return unless (mch = setup_file.content.match(regex))

"[#{mch.post_match[0..closing_bracket_index(mch.post_match, '[')]}"
end

sig { params(regex: Regexp).returns(T.nilable(String)) }
def get_regexed_req_dict(regex)
return unless (mch = setup_file.content.match(regex))

"{#{mch.post_match[0..closing_bracket_index(mch.post_match, '{')]}"
end

sig { params(string: String, bracket: String).returns(Integer) }
def closing_bracket_index(string, bracket)
closes_required = 1

Expand All @@ -165,6 +170,7 @@ def closing_bracket_index(string, bracket)
0
end

sig { params(name: String, extras: T::Array[String]).returns(String) }
def normalised_name(name, extras)
NameNormaliser.normalise_including_extras(name, extras)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Dependabot
module Python
class FileUpdater
class PyprojectPreparer
extend T::Sig
def initialize(pyproject_content:, lockfile: nil)
@pyproject_content = pyproject_content
@lockfile = lockfile
Expand Down Expand Up @@ -48,6 +49,7 @@ def update_python_requirement(requirement)
TomlRB.dump(pyproject_object)
end

sig { returns(String) }
def sanitize
# {{ name }} syntax not allowed
pyproject_content
Expand Down Expand Up @@ -111,6 +113,7 @@ def locked_details(dep_name)
.find { |d| d["name"] == normalise(dep_name) }
end

sig { params(name: String).returns(String) }
def normalise(name)
NameNormaliser.normalise(name)
end
Expand Down
Loading
Loading