From f4ce4107e842ce19b211542f094ab31c1680b98a Mon Sep 17 00:00:00 2001
From: Mark Allen <markhallen@gmail.com>
Date: Tue, 4 Feb 2025 15:05:25 +0000
Subject: [PATCH] Strict type logger formats

---
 updater/lib/dependabot/logger/formats.rb      | 28 +++++++----
 .../dependabot/logger/basic_formatter_spec.rb | 23 +++++++++
 .../dependabot/logger/job_formatter_spec.rb   | 50 +++++++++++++++++++
 3 files changed, 92 insertions(+), 9 deletions(-)
 create mode 100644 updater/spec/dependabot/logger/basic_formatter_spec.rb
 create mode 100644 updater/spec/dependabot/logger/job_formatter_spec.rb

diff --git a/updater/lib/dependabot/logger/formats.rb b/updater/lib/dependabot/logger/formats.rb
index cb8288bfc9c..d49fa483259 100644
--- a/updater/lib/dependabot/logger/formats.rb
+++ b/updater/lib/dependabot/logger/formats.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strong
 # frozen_string_literal: true
 
 require "logger"
@@ -10,19 +10,31 @@ module Logger
     TIME_FORMAT = "%Y/%m/%d %H:%M:%S"
 
     class BasicFormatter < ::Logger::Formatter
+      extend T::Sig
+
+      sig do
+        params(severity: String, _datetime: T.nilable(Time), _progname: T.nilable(String), msg: T.nilable(String))
+          .returns(String)
+      end
       def call(severity, _datetime, _progname, msg)
         "#{Time.now.strftime(TIME_FORMAT)} #{severity} #{msg2str(msg)}\n"
       end
     end
 
     class JobFormatter < ::Logger::Formatter
+      extend T::Sig
       CLI_ID = "cli"
       UNKNOWN_ID = "unknown_id"
 
+      sig { params(job_id: T.nilable(String)).void }
       def initialize(job_id)
         @job_id = job_id
       end
 
+      sig do
+        params(severity: String, _datetime: T.nilable(Time), _progname: T.nilable(String), msg: T.nilable(String))
+          .returns(String)
+      end
       def call(severity, _datetime, _progname, msg)
         [
           Time.now.strftime(TIME_FORMAT),
@@ -34,15 +46,13 @@ def call(severity, _datetime, _progname, msg)
 
       private
 
+      sig { returns(T.nilable(String)) }
       def job_prefix
-        return @job_prefix if defined? @job_prefix
-        # The dependabot/cli tool uses a placeholder value since it does not
-        # have an actual Job ID issued by the service.
-        #
-        # Let's just omit the prefix if this is the case.
-        return @job_prefix = nil if @job_id == CLI_ID
-
-        @job_prefix = "<job_#{@job_id || UNKNOWN_ID}>"
+        @job_prefix ||= T.let(begin
+          return nil if @job_id == CLI_ID
+
+          "<job_#{@job_id || UNKNOWN_ID}>"
+        end, T.nilable(String))
       end
     end
   end
diff --git a/updater/spec/dependabot/logger/basic_formatter_spec.rb b/updater/spec/dependabot/logger/basic_formatter_spec.rb
new file mode 100644
index 00000000000..12460567ca5
--- /dev/null
+++ b/updater/spec/dependabot/logger/basic_formatter_spec.rb
@@ -0,0 +1,23 @@
+# typed: true
+# frozen_string_literal: true
+
+require "spec_helper"
+require "dependabot/logger/formats"
+
+RSpec.describe Dependabot::Logger::BasicFormatter do
+  describe "#call" do
+    it "returns a formatted log line" do
+      formatter = described_class.new
+      log_line = formatter.call("INFO", Time.now, "progname", "msg")
+
+      expect(log_line).to match(%r{\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} INFO msg\n})
+    end
+
+    it "returns a formatted log line with only a severity" do
+      formatter = described_class.new
+      log_line = formatter.call("ERROR", nil, nil, nil)
+
+      expect(log_line).to match(%r{\d{4}\/\d{2}\/\d{2} \d{2}:\d{2}:\d{2} ERROR nil\n})
+    end
+  end
+end
diff --git a/updater/spec/dependabot/logger/job_formatter_spec.rb b/updater/spec/dependabot/logger/job_formatter_spec.rb
new file mode 100644
index 00000000000..b596fe118fd
--- /dev/null
+++ b/updater/spec/dependabot/logger/job_formatter_spec.rb
@@ -0,0 +1,50 @@
+# typed: true
+# frozen_string_literal: true
+
+require "spec_helper"
+require "dependabot/logger/formats"
+
+RSpec.describe Dependabot::Logger::JobFormatter do
+  describe "#new" do
+    it "returns a formatter when provided a job_id" do
+      formatter = described_class.new("job_id")
+      expect(formatter).to be_a(described_class)
+    end
+
+    it "returns a formatter when provided a nil job_id" do
+      formatter = described_class.new(nil)
+      expect(formatter).to be_a(described_class)
+    end
+  end
+
+  describe "#call" do
+    let(:job_id) { "job_id" }
+    let(:formatter) { described_class.new(job_id) }
+
+    it "returns a formatted log line with a job_id" do
+      log_line = formatter.call("INFO", Time.now, "progname", "msg")
+
+      expect(log_line).to match(%r{\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} INFO <job_job_id> msg\n})
+    end
+
+    it "returns a formatted log line with a nil job_id" do
+      formatter = described_class.new(nil)
+      log_line = formatter.call("INFO", Time.now, "progname", "msg")
+
+      expect(log_line).to match(%r{\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} INFO <job_unknown_id> msg\n})
+    end
+
+    it "returns a formatted log line with only a severity" do
+      log_line = formatter.call("ERROR", nil, nil, nil)
+
+      expect(log_line).to match(%r{\d{4}\/\d{2}\/\d{2} \d{2}:\d{2}:\d{2} ERROR <job_job_id> nil\n})
+    end
+
+    it "returns a formatted log line when using the CLI" do
+      formatter = described_class.new("cli")
+      log_line = formatter.call("ERROR", nil, nil, nil)
+
+      expect(log_line).to match(%r{\d{4}\/\d{2}\/\d{2} \d{2}:\d{2}:\d{2} ERROR nil\n})
+    end
+  end
+end