Merge pull request #7245 from dependabot/deivid-rodriguez/refactor-pr…

…eparing-json-files

Refactor preparing `package.json` files

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb

@@ -513,16 +513,18 @@ def write_temporary_dependency_files(update_package_json: true)
                 file.content
               end
 
+            package_json_preparer = package_json_preparer(updated_content)
+
             # TODO: Figure out if we need to lock git deps for npm 7 and can
             # start deprecating this hornets nest
             #
             # NOTE: When updating a package-lock.json we have to manually lock
             # all git dependencies, otherwise npm will (unhelpfully) update them
             updated_content = lock_git_deps(updated_content)
-            updated_content = replace_ssh_sources(updated_content)
+            updated_content = package_json_preparer.replace_ssh_sources(updated_content)
             updated_content = lock_deps_with_latest_reqs(updated_content)
 
-            updated_content = sanitized_package_json_content(updated_content)
+            updated_content = package_json_preparer.remove_invalid_characters(updated_content)
 
             File.write(file.name, updated_content)
           end
@@ -614,35 +616,12 @@ def lock_deps_with_latest_reqs(content)
           JSON.pretty_generate(json, indent: indent)
         end
 
-        def replace_ssh_sources(content)
-          updated_content = content
-
-          git_ssh_requirements_to_swap.each do |req|
-            new_req = req.gsub(%r{git\+ssh://git@(.*?)[:/]}, 'https://\1/')
-            updated_content = updated_content.gsub(req, new_req)
-          end
-
-          updated_content
-        end
-
         def git_ssh_requirements_to_swap
           return @git_ssh_requirements_to_swap if @git_ssh_requirements_to_swap
 
-          @git_ssh_requirements_to_swap = []
-
-          package_files.each do |file|
-            NpmAndYarn::FileParser::DEPENDENCY_TYPES.each do |t|
-              JSON.parse(file.content).fetch(t, {}).each do |_, requirement|
-                next unless requirement.is_a?(String)
-                next unless requirement.start_with?("git+ssh:")
-
-                req = requirement.split("#").first
-                @git_ssh_requirements_to_swap << req
-              end
-            end
+          @git_ssh_requirements_to_swap = package_files.flat_map do |file|
+            package_json_preparer(file.content).swapped_ssh_requirements
           end
-
-          @git_ssh_requirements_to_swap
         end
 
         def post_process_npm_lockfile(updated_lockfile_content)
@@ -841,6 +820,14 @@ def updated_package_json_content(file)
             ).updated_package_json.content
         end
 
+        def package_json_preparer(content)
+          @package_json_preparer ||= {}
+          @package_json_preparer[content] ||=
+            PackageJsonPreparer.new(
+              package_json_content: content
+            )
+        end
+
         def npmrc_disables_lockfile?
           npmrc_content.match?(/^package-lock\s*=\s*false/)
         end
@@ -851,13 +838,6 @@ def npm8?
           @npm8 = Dependabot::NpmAndYarn::Helpers.npm_version(lockfile.content) == "npm8"
         end
 
-        def sanitized_package_json_content(content)
-          content.
-            gsub(/\{\{[^\}]*?\}\}/, "something"). # {{ nm }} syntax not allowed
-            gsub(/(?<!\\)\\ /, " ").          # escaped whitespace not allowed
-            gsub(%r{^\s*//.*}, " ")           # comments are not allowed
-        end
-
         def sanitize_package_name(package_name)
           package_name.gsub("%2f", "/").gsub("%2F", "/")
         end

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/package_json_preparer.rb

@@ -48,7 +48,7 @@ def remove_workspace_path_prefixes(content)
 
           paths_array.each { |path| path.gsub!(%r{^\./}, "") }
 
-          json.to_json
+          JSON.pretty_generate(json)
         end
 
         def remove_invalid_characters(content)

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater.rb

@@ -17,6 +17,7 @@ class FileUpdater
       class YarnLockfileUpdater
         require_relative "npmrc_builder"
         require_relative "package_json_updater"
+        require_relative "package_json_preparer"
 
         def initialize(dependencies:, dependency_files:, repo_contents_path:, credentials:)
           @dependencies = dependencies
@@ -357,13 +358,7 @@ def write_temporary_dependency_files(yarn_lock, update_package_json: true)
                 file.content
               end
 
-            updated_content = replace_ssh_sources(updated_content)
-
-            # A bug prevents Yarn recognising that a directory is part of a
-            # workspace if it is specified with a `./` prefix.
-            updated_content = remove_workspace_path_prefixes(updated_content)
-
-            updated_content = sanitized_package_json_content(updated_content)
+            updated_content = package_json_preparer(updated_content).prepared_content
             File.write(file.name, updated_content)
           end
 
@@ -395,60 +390,12 @@ def write_lockfiles
           end
         end
 
-        def replace_ssh_sources(content)
-          updated_content = content
-
-          git_ssh_requirements_to_swap.each do |req|
-            new_req = req.gsub(%r{git\+ssh://git@(.*?)[:/]}, 'https://\1/')
-            updated_content = updated_content.gsub(req, new_req)
-          end
-
-          updated_content
-        end
-
-        def remove_workspace_path_prefixes(content)
-          json = JSON.parse(content)
-          return content unless json.key?("workspaces")
-
-          workspace_object = json.fetch("workspaces")
-          paths_array =
-            if workspace_object.is_a?(Hash)
-              workspace_object.values_at("packages", "nohoist").
-                flatten.compact
-            elsif workspace_object.is_a?(Array) then workspace_object
-            else
-              raise "Unexpected workspace object"
-            end
-
-          paths_array.each { |path| path.gsub!(%r{^\./}, "") }
-
-          json.to_json
-        end
-
         def git_ssh_requirements_to_swap
           return @git_ssh_requirements_to_swap if @git_ssh_requirements_to_swap
 
-          git_dependencies =
-            dependencies.
-            select do |dep|
-              dep.requirements.any? { |r| r.dig(:source, :type) == "git" }
-            end
-
-          @git_ssh_requirements_to_swap = []
-
-          package_files.each do |file|
-            NpmAndYarn::FileParser::DEPENDENCY_TYPES.each do |t|
-              JSON.parse(file.content).fetch(t, {}).each do |nm, requirement|
-                next unless git_dependencies.map(&:name).include?(nm)
-                next unless requirement.start_with?("git+ssh:")
-
-                req = requirement.split("#").first
-                @git_ssh_requirements_to_swap << req
-              end
-            end
+          @git_ssh_requirements_to_swap = package_files.flat_map do |file|
+            package_json_preparer(file.content).swapped_ssh_requirements
           end
-
-          @git_ssh_requirements_to_swap
         end
 
         def post_process_yarn_lockfile(lockfile_content)
@@ -538,12 +485,18 @@ def npmrc_content
         end
 
         def updated_package_json_content(file)
-          @updated_package_json_content ||= {}
-          @updated_package_json_content[file.name] ||=
-            PackageJsonUpdater.new(
-              package_json: file,
-              dependencies: top_level_dependencies
-            ).updated_package_json.content
+          PackageJsonUpdater.new(
+            package_json: file,
+            dependencies: top_level_dependencies
+          ).updated_package_json.content
+        end
+
+        def package_json_preparer(content)
+          @package_json_preparer ||= {}
+          @package_json_preparer[content] ||=
+            PackageJsonPreparer.new(
+              package_json_content: content
+            )
         end
 
         def npmrc_disables_lockfile?
@@ -575,13 +528,6 @@ def yarnrc_content
           ).yarnrc_content
         end
 
-        def sanitized_package_json_content(content)
-          content.
-            gsub(/\{\{[^\}]*?\}\}/, "something"). # {{ nm }} syntax not allowed
-            gsub(/(?<!\\)\\ /, " ").          # escaped whitespace not allowed
-            gsub(%r{^\s*//.*}, " ")           # comments are not allowed
-        end
-
         def sanitize_package_name(package_name)
           package_name.gsub("%2f", "/").gsub("%2F", "/")
         end