From fd35e2570119e0f6b5e3c5db97293b95308197d1 Mon Sep 17 00:00:00 2001 From: Rob Aiken Date: Wed, 29 Jan 2025 16:01:44 +0000 Subject: [PATCH 1/5] adding sorbet types for elm file parser --- elm/lib/dependabot/elm/file_parser.rb | 56 ++++++++++++++++++--------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/elm/lib/dependabot/elm/file_parser.rb b/elm/lib/dependabot/elm/file_parser.rb index 0b61d1a69bd..22c8478a81a 100644 --- a/elm/lib/dependabot/elm/file_parser.rb +++ b/elm/lib/dependabot/elm/file_parser.rb @@ -1,4 +1,4 @@ -# typed: true +# typed: strict # frozen_string_literal: true require "dependabot/dependency" @@ -12,10 +12,17 @@ module Dependabot module Elm class FileParser < Dependabot::FileParsers::Base + extend T::Sig + require "dependabot/file_parsers/base/dependency_set" - DEPENDENCY_TYPES = %w(dependencies test-dependencies).freeze + DEPENDENCY_TYPES = T.let(%w(dependencies test-dependencies).freeze, T::Array[String]) + MANIFEST_FILE = T.let("elm.json", String) + ELM_VERSION_KEY = T.let("elm-version", String) + ECOSYSTEM = T.let("elm", String) + DEFAULT_ELM_VERSION = T.let("0.19.1", String) + sig { override.returns(T::Array[Dependabot::Dependency]) } def parse dependency_set = DependencySet.new @@ -78,47 +85,43 @@ def extract_version_requirement(field) Dependabot::Elm::Requirement.new(content) end - # Extracts the version content (e.g., "1.9.1" or "<= 1.9.1") and parses it to return only the version part sig { params(field: String).returns(T.nilable(String)) } def extract_version(field) version_content = extract_version_content(field) return nil unless version_content - # Extract only the version part (e.g., "1.9.1") from the string version_match = version_content.match(/(\d+\.\d+\.\d+)/) version_match ? version_match[1] : nil end sig { params(field: String).returns(T.nilable(String)) } def extract_version_content(field) - parsed_version = parsed_elm_json.fetch(field, nil) + parsed_version = T.must(parsed_elm_json).fetch(field, nil) return if parsed_version.nil? || parsed_version.empty? parsed_version end - # For docs on elm.json, see: - # https://github.com/elm/compiler/blob/master/docs/elm.json/application.md - # https://github.com/elm/compiler/blob/master/docs/elm.json/package.md + sig { returns(Dependabot::FileParsers::Base::DependencySet) } def elm_json_dependencies dependency_set = DependencySet.new DEPENDENCY_TYPES.each do |dep_type| if repo_type == "application" - dependencies_hash = parsed_elm_json.fetch(dep_type, {}) - dependencies_hash.fetch("direct", {}).each do |name, req| + dependencies_hash = T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), T::Hash[String, T.untyped]) + T.cast(dependencies_hash.fetch("direct", {}), T::Hash[String, String]).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: true ) end - dependencies_hash.fetch("indirect", {}).each do |name, req| + T.cast(dependencies_hash.fetch("indirect", {}), T::Hash[String, String]).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: false ) end elsif repo_type == "package" - parsed_elm_json.fetch(dep_type, {}).each do |name, req| + T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), T::Hash[String, String]).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: true ) @@ -131,13 +134,21 @@ def elm_json_dependencies dependency_set end + sig do + params( + name: String, + group: String, + requirement: String, + direct: T::Boolean + ).returns(Dependabot::Dependency) + end def build_elm_json_dependency(name:, group:, requirement:, direct:) requirements = [{ - requirement: requirement, - groups: [group], - source: nil, - file: MANIFEST_FILE - }] + requirement: requirement, + groups: [group], + source: nil, + file: MANIFEST_FILE + }] Dependency.new( name: name, @@ -149,7 +160,7 @@ def build_elm_json_dependency(name:, group:, requirement:, direct:) sig { returns(String) } def repo_type - parsed_elm_json.fetch("type") + T.must(parsed_elm_json).fetch("type") end sig { override.void } @@ -159,6 +170,11 @@ def check_required_files raise "No #{MANIFEST_FILE}!" end + sig do + params( + version_requirement: T.nilable(T.any(String, T::Array[T.nilable(String)])) + ).returns(T.nilable(Gem::Version)) + end def version_for(version_requirement) req = Dependabot::Elm::Requirement.new(version_requirement) @@ -167,12 +183,14 @@ def version_for(version_requirement) req.requirements.first.last end + sig { returns(T.nilable(T::Hash[String, T.untyped])) } def parsed_elm_json - @parsed_elm_json ||= JSON.parse(elm_json.content) + @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), T.nilable(T::Hash[String, T.untyped])) rescue JSON::ParserError raise Dependabot::DependencyFileNotParseable, elm_json&.path || MANIFEST_FILE end + sig { returns(T.nilable(Dependabot::DependencyFile)) } def elm_json @elm_json ||= T.let( get_original_file(MANIFEST_FILE), From 895f38f0fa67972d1ace4ecd04dbdbc122c1f6c3 Mon Sep 17 00:00:00 2001 From: Rob Aiken Date: Mon, 3 Feb 2025 16:01:47 +0000 Subject: [PATCH 2/5] adding types --- elm/lib/dependabot/elm/file_parser.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/elm/lib/dependabot/elm/file_parser.rb b/elm/lib/dependabot/elm/file_parser.rb index 22c8478a81a..ade6afe2539 100644 --- a/elm/lib/dependabot/elm/file_parser.rb +++ b/elm/lib/dependabot/elm/file_parser.rb @@ -109,13 +109,14 @@ def elm_json_dependencies DEPENDENCY_TYPES.each do |dep_type| if repo_type == "application" - dependencies_hash = T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), T::Hash[String, T.untyped]) - T.cast(dependencies_hash.fetch("direct", {}), T::Hash[String, String]).each do |name, req| + dependencies_hash = T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), + T::Hash[String, T::Hash[String, String]]) + dependencies_hash.fetch("direct", {}).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: true ) end - T.cast(dependencies_hash.fetch("indirect", {}), T::Hash[String, String]).each do |name, req| + dependencies_hash.fetch("indirect", {}).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: false ) @@ -144,11 +145,11 @@ def elm_json_dependencies end def build_elm_json_dependency(name:, group:, requirement:, direct:) requirements = [{ - requirement: requirement, - groups: [group], - source: nil, - file: MANIFEST_FILE - }] + requirement: requirement, + groups: [group], + source: nil, + file: MANIFEST_FILE + }] Dependency.new( name: name, @@ -183,9 +184,10 @@ def version_for(version_requirement) req.requirements.first.last end - sig { returns(T.nilable(T::Hash[String, T.untyped])) } + sig { returns(T.nilable(T::Hash[String, T.any(String, T::Boolean, NilClass)])) } def parsed_elm_json - @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), T.nilable(T::Hash[String, T.untyped])) + @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), + T.nilable(T::Hash[String, T.any(String, T::Boolean, NilClass)])) rescue JSON::ParserError raise Dependabot::DependencyFileNotParseable, elm_json&.path || MANIFEST_FILE end From 96d640f38f32bcff9dbe0eaf0e842fdd6675af62 Mon Sep 17 00:00:00 2001 From: Rob Aiken Date: Mon, 3 Feb 2025 16:11:21 +0000 Subject: [PATCH 3/5] fix sorbet type errors --- elm/lib/dependabot/elm/file_parser.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/elm/lib/dependabot/elm/file_parser.rb b/elm/lib/dependabot/elm/file_parser.rb index ade6afe2539..01559426274 100644 --- a/elm/lib/dependabot/elm/file_parser.rb +++ b/elm/lib/dependabot/elm/file_parser.rb @@ -96,9 +96,10 @@ def extract_version(field) sig { params(field: String).returns(T.nilable(String)) } def extract_version_content(field) - parsed_version = T.must(parsed_elm_json).fetch(field, nil) - - return if parsed_version.nil? || parsed_version.empty? + parsed_version = parsed_elm_json.fetch(field, nil) + return nil if parsed_version.nil? + return nil unless parsed_version.is_a?(String) + return nil if parsed_version.empty? parsed_version end @@ -109,7 +110,7 @@ def elm_json_dependencies DEPENDENCY_TYPES.each do |dep_type| if repo_type == "application" - dependencies_hash = T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), + dependencies_hash = T.cast(parsed_elm_json.fetch(dep_type, {}), T::Hash[String, T::Hash[String, String]]) dependencies_hash.fetch("direct", {}).each do |name, req| dependency_set << build_elm_json_dependency( @@ -122,7 +123,7 @@ def elm_json_dependencies ) end elsif repo_type == "package" - T.cast(T.must(parsed_elm_json).fetch(dep_type, {}), T::Hash[String, String]).each do |name, req| + T.cast(parsed_elm_json.fetch(dep_type, {}), T::Hash[String, String]).each do |name, req| dependency_set << build_elm_json_dependency( name: name, group: dep_type, requirement: req, direct: true ) @@ -161,7 +162,8 @@ def build_elm_json_dependency(name:, group:, requirement:, direct:) sig { returns(String) } def repo_type - T.must(parsed_elm_json).fetch("type") + type = parsed_elm_json.fetch("type") + T.must(type.is_a?(String) ? type : nil) end sig { override.void } @@ -184,10 +186,10 @@ def version_for(version_requirement) req.requirements.first.last end - sig { returns(T.nilable(T::Hash[String, T.any(String, T::Boolean, NilClass)])) } + sig { returns(T::Hash[String, T.any(String, T::Hash[String, T.any(String, T::Hash[String, String])])]) } def parsed_elm_json - @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), - T.nilable(T::Hash[String, T.any(String, T::Boolean, NilClass)])) + @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), T.nilable(T::Hash[String, T.any(String, T::Hash[String, T.any(String, T::Hash[String, String])])])) + rescue JSON::ParserError raise Dependabot::DependencyFileNotParseable, elm_json&.path || MANIFEST_FILE end From 050e88d4da8761aecb5cb0163bbfd0a7edb0eb8e Mon Sep 17 00:00:00 2001 From: Rob Aiken Date: Mon, 3 Feb 2025 16:17:06 +0000 Subject: [PATCH 4/5] lint --- elm/lib/dependabot/elm/file_parser.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/elm/lib/dependabot/elm/file_parser.rb b/elm/lib/dependabot/elm/file_parser.rb index 01559426274..f6cb37ef2ae 100644 --- a/elm/lib/dependabot/elm/file_parser.rb +++ b/elm/lib/dependabot/elm/file_parser.rb @@ -188,8 +188,10 @@ def version_for(version_requirement) sig { returns(T::Hash[String, T.any(String, T::Hash[String, T.any(String, T::Hash[String, String])])]) } def parsed_elm_json - @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), T.nilable(T::Hash[String, T.any(String, T::Hash[String, T.any(String, T::Hash[String, String])])])) - + @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), + T.nilable(T::Hash[String, + T.any(String, + T::Hash[String, T.any(String, T::Hash[String, String])])])) rescue JSON::ParserError raise Dependabot::DependencyFileNotParseable, elm_json&.path || MANIFEST_FILE end From 92cfe0a0cea5575efb662099953ce681852a2f25 Mon Sep 17 00:00:00 2001 From: Rob Aiken Date: Thu, 27 Feb 2025 13:10:13 +0000 Subject: [PATCH 5/5] taking a more conservative approach --- elm/lib/dependabot/elm/file_parser.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/elm/lib/dependabot/elm/file_parser.rb b/elm/lib/dependabot/elm/file_parser.rb index f6cb37ef2ae..be8ee237308 100644 --- a/elm/lib/dependabot/elm/file_parser.rb +++ b/elm/lib/dependabot/elm/file_parser.rb @@ -162,8 +162,7 @@ def build_elm_json_dependency(name:, group:, requirement:, direct:) sig { returns(String) } def repo_type - type = parsed_elm_json.fetch("type") - T.must(type.is_a?(String) ? type : nil) + parsed_elm_json.fetch("type") end sig { override.void } @@ -186,12 +185,9 @@ def version_for(version_requirement) req.requirements.first.last end - sig { returns(T::Hash[String, T.any(String, T::Hash[String, T.any(String, T::Hash[String, String])])]) } + sig { returns(T.untyped) } def parsed_elm_json - @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), - T.nilable(T::Hash[String, - T.any(String, - T::Hash[String, T.any(String, T::Hash[String, String])])])) + @parsed_elm_json ||= T.let(JSON.parse(T.must(T.must(elm_json).content)), T.untyped) rescue JSON::ParserError raise Dependabot::DependencyFileNotParseable, elm_json&.path || MANIFEST_FILE end