From 803b2bc240a5735a6647eaf60fe42dff399442be Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Sat, 17 Oct 2026 12:36:06 +1300 Subject: [PATCH 1/2] Remove the mailer metadata that never collected anything add_current_tags_to_log_data asked ActiveSupport::TaggedLogging and ActiveJob::Logging for current_tags and job_id, which neither has (the tags belong to each tagged logger's formatter, and the job ID to each job), and its fallback read a thread-local key Rails doesn't use. So it never added tags or a job ID to a mailer's log lines; its tests passed only because they defined those methods first. --- .../action_mailer/metadata_collection.rb | 20 -------------- .../action_mailer/metadata_collection_test.rb | 27 ------------------- 2 files changed, 47 deletions(-) diff --git a/lib/log_struct/integrations/action_mailer/metadata_collection.rb b/lib/log_struct/integrations/action_mailer/metadata_collection.rb index c15120a2..2f83c43c 100644 --- a/lib/log_struct/integrations/action_mailer/metadata_collection.rb +++ b/lib/log_struct/integrations/action_mailer/metadata_collection.rb @@ -25,9 +25,6 @@ def self.add_message_metadata(mailer, log_data) def self.add_context_metadata(mailer, log_data) # Add account ID information if available (but not user email) extract_ids_to_log_data(mailer, log_data) - - # Add any current tags from ActiveJob or ActionMailer - add_current_tags_to_log_data(log_data) end sig { params(mailer: T.untyped, log_data: T::Hash[Symbol, T.untyped]).void } @@ -43,23 +40,6 @@ def self.extract_ids_to_log_data(mailer, log_data) log_data[log_key] = obj.id if obj.respond_to?(:id) end end - - sig { params(log_data: T::Hash[Symbol, T.untyped]).void } - def self.add_current_tags_to_log_data(log_data) - # Get current tags from thread-local storage or ActiveSupport::TaggedLogging - tags = if ::ActiveSupport::TaggedLogging.respond_to?(:current_tags) - T.unsafe(::ActiveSupport::TaggedLogging).current_tags - else - Thread.current[:activesupport_tagged_logging_tags] || [] - end - log_data[:tags] = tags if tags.present? - - # Get job_id from ActiveJob if available - if defined?(::ActiveJob::Logging) && ::ActiveJob::Logging.respond_to?(:job_id) && - T.unsafe(::ActiveJob::Logging).job_id.present? - log_data[:job_id] = T.unsafe(::ActiveJob::Logging).job_id - end - end end end end diff --git a/test/log_struct/integrations/action_mailer/metadata_collection_test.rb b/test/log_struct/integrations/action_mailer/metadata_collection_test.rb index c1bec49c..832c33d4 100644 --- a/test/log_struct/integrations/action_mailer/metadata_collection_test.rb +++ b/test/log_struct/integrations/action_mailer/metadata_collection_test.rb @@ -23,31 +23,4 @@ class ActionMailerMetadataCollectionTest < ActiveSupport::TestCase assert_equal 0, data[:attachment_count] end - - test "add_context_metadata gathers tags when available" do - data = {} - - # Stub ActiveSupport::TaggedLogging.current_tags - ActiveSupport::TaggedLogging.define_singleton_method(:current_tags) { ["req:abc"] } - - MetadataCollection.add_context_metadata(Object.new, data) - - assert_equal ["req:abc"], data[:tags] - # Note: request_id is now handled centrally by serialize_common via LogStruct::Current - end - - test "add_context_metadata includes job_id when ActiveJob::Logging.job_id is available" do - data = {} - # Ensure module exists - unless defined?(::ActiveJob::Logging) - module ::ActiveJob - module Logging; end - end - end - # Define job_id method on the module's singleton class - ::ActiveJob::Logging.define_singleton_method(:job_id) { "jid-1" } - MetadataCollection.add_context_metadata(Object.new, data) - - assert_equal "jid-1", data[:job_id] - end end From 973b666a226e4242c2eb7152d4ea87b80b14d5c7 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Sat, 17 Oct 2026 12:36:13 +1300 Subject: [PATCH 2/2] Use T.unsafe only where Sorbet can't type the code Most uses wrapped values that are already T.untyped (const_get, instance_variable_get, an untyped parameter), so they did nothing. The others now have typed alternatives: an override saying ActiveSupport.on_load's block runs with an untyped self, a shim for Shrine's subscribe (added by its instrumentation plugin), requires_ancestor in the Rails server command patch and the test line filter, and typed Arrays in two tests. The tagged logging formatter calls current_tags directly: Rails 7.1, the oldest version LogStruct supports, always has it. T.unsafe stays for a splat Sorbet can't size, a duck-typed call after respond_to?, Minitest::Mock and a test that passes a wrong type on purpose. AGENTS.md now lists what to try first and these cases, in place of a ban that pushed code towards equivalent workarounds. --- .cursor/rules/sorbet.mdc | 6 +++--- AGENTS.md | 13 ++++++++++++- lib/log_struct/integrations/action_mailer.rb | 2 +- .../integrations/action_mailer/error_handling.rb | 2 +- lib/log_struct/integrations/ahoy.rb | 2 +- lib/log_struct/integrations/dotenv.rb | 16 ++++++++-------- lib/log_struct/integrations/puma.rb | 16 ++++++++-------- lib/log_struct/integrations/shrine.rb | 6 +++--- .../active_support/tagged_logging/formatter.rb | 5 ++--- lib/log_struct/rails_boot_banner_silencer.rb | 6 ++++-- lib/log_struct/semantic_logger/formatter.rb | 2 +- lib/log_struct/semantic_logger/logger.rb | 2 +- lib/log_struct/semantic_logger/setup.rb | 4 ++-- .../app/controllers/logging_controller.rb | 4 ++-- sorbet/rbi/overrides/active_support.rbi | 7 +++++++ sorbet/rbi/shims/shrine.rbi | 6 ++++++ test/log_struct/configuration_test.rb | 4 ++-- test/log_struct/error_handling_test.rb | 4 ++-- test/log_struct/formatter_test.rb | 5 +---- test/log_struct/log/sql_test.rb | 1 + test/log_struct/multi_error_reporter_test.rb | 5 +++-- .../rails_boot_banner_silencer_test.rb | 4 ++-- test/support/filter_by_line.rb | 6 +++--- 23 files changed, 76 insertions(+), 52 deletions(-) diff --git a/.cursor/rules/sorbet.mdc b/.cursor/rules/sorbet.mdc index b0ccdb0f..12ff6cbc 100644 --- a/.cursor/rules/sorbet.mdc +++ b/.cursor/rules/sorbet.mdc @@ -1,6 +1,6 @@ --- description: Working with Sorbet and Rails constants -globs: +globs: alwaysApply: true --- - `T::Sig` is not available globally, we must extend it whenever we need it (we're building a library, not an application.) @@ -13,7 +13,7 @@ alwaysApply: true - Use Sorbet type annotations for all methods - Ensure all files have the appropriate `# typed:` annotation -- **NEVER use `T.unsafe` calls**. Instead, properly type your code or use appropriate type assertions with `T.let` or `T.cast` when absolutely necessary. +- Use `T.unsafe` only in the cases listed in AGENTS.md (Code Style), with a comment saying why. Properly type your code first, or use `T.let` or `T.cast`. - `T.untyped` is generally ok for Hash values when they come from unknown sources. - When dealing with external libraries, create proper type signatures or use extension methods rather than resorting to `T.unsafe`. - **NEVER use `class.name`** anywhere - this is a Sorbet quirk that hides the `name` method from all base classes. Prefer just using Classes themselves as the type. `"#{class}"` will automatically call `.to_s`. Similarly, `to_json` will automatically call `.to_s` - but you can call `.to_s` manually if you really need it. @@ -32,7 +32,7 @@ end # BAD SomeGem.configure do - T.unsafe(self).add_option "value" # NEVER do this! + T.unsafe(self).add_option "value" # use T.bind instead end ``` diff --git a/AGENTS.md b/AGENTS.md index cb2a1487..e3ebe828 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,7 +144,18 @@ You do not need to check if these are defined with `defined?` - they are guarant - Method signatures must include proper return types - Use `::` prefixes for external modules (Rails/third-party gems) - `T.must` may be used very sparingly but only in tests - e.g. `T.must(log_file.path)` -- `T.unsafe` must NEVER be used. Use proper typing or `T.let`/`T.cast` when necessary +- `T.unsafe` turns off type checking for one value. Use it only where Sorbet can't type the code another way, + with a short comment saying why. Try these first: + - Nothing: `const_get`, `instance_variable_get`, `send`/`public_send` and `JSON.parse` already return + `T.untyped`, so wrapping their results adds nothing. + - Narrowing with `is_a?` or `case`, or `T.cast`/`T.let` when you know the type. + - `requires_ancestor` in a module that's included in or prepended to another class. + - `T.bind(self, SomeClass)` in a block that runs with a known `self`, or an override in `sorbet/rbi/overrides/` + when the block's `self` varies (`ActiveSupport.on_load`). + - A shim in `sorbet/rbi/shims/` for an optional gem's API that Tapioca can't see (Shrine's plugins). + + What's left: splatting an Array or Hash whose size Sorbet doesn't know, a duck-typed call after `respond_to?` on + another library's object, a `Minitest::Mock`, and a test that passes a wrong type on purpose. - Don't call `def` inside a method definition in tests (or anywhere). Use mocks or stubs. - For modules included in other classes, use `requires_ancestor` - Custom type overrides belong in `sorbet/rbi/overrides/` diff --git a/lib/log_struct/integrations/action_mailer.rb b/lib/log_struct/integrations/action_mailer.rb index 1a1f69e1..8f4d23bf 100644 --- a/lib/log_struct/integrations/action_mailer.rb +++ b/lib/log_struct/integrations/action_mailer.rb @@ -32,7 +32,7 @@ def self.setup(config) # if ActionMailer::Base is already loaded. ActiveSupport.on_load(:action_mailer) do # Silence default ActionMailer logs (we use our own structured logging) - T.unsafe(self).logger = ::Logger.new(File::NULL) + self.logger = ::Logger.new(File::NULL) prepend LogStruct::Integrations::ActionMailer::EventLogging prepend LogStruct::Integrations::ActionMailer::ErrorHandling diff --git a/lib/log_struct/integrations/action_mailer/error_handling.rb b/lib/log_struct/integrations/action_mailer/error_handling.rb index 39469bc0..3c11f3b5 100644 --- a/lib/log_struct/integrations/action_mailer/error_handling.rb +++ b/lib/log_struct/integrations/action_mailer/error_handling.rb @@ -264,7 +264,7 @@ def build_exception_data(error, level, context_data) sig { params(error: StandardError).returns(String) } def recipients(error) - # Extract recipient info if available + # Some delivery errors (Postmark's, for example) have the recipients if error.respond_to?(:recipients) && T.unsafe(error).recipients.present? T.unsafe(error).recipients.join(", ") else diff --git a/lib/log_struct/integrations/ahoy.rb b/lib/log_struct/integrations/ahoy.rb index ee3c2ef6..24b5f2fe 100644 --- a/lib/log_struct/integrations/ahoy.rb +++ b/lib/log_struct/integrations/ahoy.rb @@ -43,7 +43,7 @@ def track(name, properties = nil, options = {}) end end - T.unsafe(::Ahoy::Tracker).prepend(mod) + ::Ahoy::Tracker.prepend(mod) end true diff --git a/lib/log_struct/integrations/dotenv.rb b/lib/log_struct/integrations/dotenv.rb index 1f89572e..c7b35b81 100644 --- a/lib/log_struct/integrations/dotenv.rb +++ b/lib/log_struct/integrations/dotenv.rb @@ -130,8 +130,8 @@ def self.setup_boot return unless defined?(::ActiveSupport::Notifications) instrumenter = if Object.const_defined?(:Dotenv) - dm = T.unsafe(Object.const_get(:Dotenv)) - dm.respond_to?(:instrumenter) ? T.unsafe(dm).instrumenter : ::ActiveSupport::Notifications + dm = Object.const_get(:Dotenv) + dm.respond_to?(:instrumenter) ? dm.instrumenter : ::ActiveSupport::Notifications else ::ActiveSupport::Notifications end @@ -183,9 +183,9 @@ def self.intercept_logger_setter! return unless Object.const_defined?(:Dotenv) # Do not intercept when LogStruct is disabled; allow original dotenv replay return unless LogStruct.enabled? - dotenv_mod = T.unsafe(Object.const_get(:Dotenv)) + dotenv_mod = Object.const_get(:Dotenv) return unless dotenv_mod.const_defined?(:Rails) - klass = T.unsafe(dotenv_mod.const_get(:Rails)) + klass = dotenv_mod.const_get(:Rails) return if klass.instance_variable_defined?(:@_logstruct_replay_patched) original = klass.instance_method(:logger=) @@ -212,15 +212,15 @@ def self.intercept_logger_setter! def self.resolve_boot_logs! # If LogStruct is disabled, do not alter dotenv behavior at all return unless LogStruct.enabled? - dotenv_mod = Object.const_defined?(:Dotenv) ? T.unsafe(Object.const_get(:Dotenv)) : nil - klass = dotenv_mod&.const_defined?(:Rails) ? T.unsafe(dotenv_mod.const_get(:Rails)) : nil + dotenv_mod = Object.const_defined?(:Dotenv) ? Object.const_get(:Dotenv) : nil + klass = dotenv_mod&.const_defined?(:Rails) ? dotenv_mod.const_get(:Rails) : nil pending_logger = nil railtie_instance = nil if klass&.respond_to?(:instance) railtie_instance = klass.instance if railtie_instance.respond_to?(:logstruct_pending_dotenv_logger) - pending_logger = T.unsafe(railtie_instance).logstruct_pending_dotenv_logger + pending_logger = railtie_instance.logstruct_pending_dotenv_logger end end @@ -242,7 +242,7 @@ def self.resolve_boot_logs! # Detach original subscriber and subscribe runtime structured if dotenv_mod&.const_defined?(:LogSubscriber) - T.unsafe(dotenv_mod.const_get(:LogSubscriber)).detach_from(:dotenv) + dotenv_mod.const_get(:LogSubscriber).detach_from(:dotenv) end LogStruct::Integrations::Dotenv.subscribe! diff --git a/lib/log_struct/integrations/puma.rb b/lib/log_struct/integrations/puma.rb index 2a9df380..76ff8b3f 100644 --- a/lib/log_struct/integrations/puma.rb +++ b/lib/log_struct/integrations/puma.rb @@ -98,16 +98,16 @@ def install_patches! rescue => e handle_integration_error(e) end - puma_mod = ::Object.const_defined?(:Puma) ? T.unsafe(::Object.const_get(:Puma)) : nil # rubocop:disable Sorbet/ConstantsFromStrings + puma_mod = ::Object.const_defined?(:Puma) ? ::Object.const_get(:Puma) : nil # rubocop:disable Sorbet/ConstantsFromStrings # rubocop:disable Sorbet/ConstantsFromStrings if puma_mod&.const_defined?(:LogWriter) - T.unsafe(::Object.const_get("Puma::LogWriter")).prepend(LogWriterPatch) + ::Object.const_get("Puma::LogWriter").prepend(LogWriterPatch) end if puma_mod&.const_defined?(:ErrorLogger) - T.unsafe(::Object.const_get("Puma::ErrorLogger")).prepend(ErrorLoggerPatch) + ::Object.const_get("Puma::ErrorLogger").prepend(ErrorLoggerPatch) end if puma_mod&.const_defined?(:Events) - ev = T.unsafe(::Object.const_get("Puma::Events")) + ev = ::Object.const_get("Puma::Events") ev.prepend(EventsPatch) end if puma_mod&.const_defined?(:Cluster) @@ -119,11 +119,11 @@ def install_patches! end # Patch Rack::Handler::Puma.run to emit lifecycle logs using options if ::Object.const_defined?(:Rack) - rack_mod = T.unsafe(::Object.const_get(:Rack)) + rack_mod = ::Object.const_get(:Rack) if rack_mod.const_defined?(:Handler) - handler_mod = T.unsafe(rack_mod.const_get(:Handler)) + handler_mod = rack_mod.const_get(:Handler) if handler_mod.const_defined?(:Puma) - handler = T.unsafe(handler_mod.const_get(:Puma)) + handler = handler_mod.const_get(:Puma) handler.singleton_class.prepend(RackHandlerPatch) end end @@ -369,7 +369,7 @@ def puts(msg) return nil end if ::Kernel.instance_variables.include?(:@stdout) - io = T.unsafe(::Kernel.instance_variable_get(:@stdout)) + io = ::Kernel.instance_variable_get(:@stdout) return io.puts(msg) end super diff --git a/lib/log_struct/integrations/shrine.rb b/lib/log_struct/integrations/shrine.rb index 6f157197..db482d3e 100644 --- a/lib/log_struct/integrations/shrine.rb +++ b/lib/log_struct/integrations/shrine.rb @@ -25,7 +25,7 @@ def self.setup(config) # Create a structured log subscriber for Shrine # ActiveSupport::Notifications::Event has name, time, end, transaction_id, payload, and duration - shrine_log_subscriber = T.unsafe(lambda do |event| + shrine_log_subscriber = lambda do |event| payload = event.payload.except(:io, :metadata, :name).dup # Map event name to Event type @@ -86,14 +86,14 @@ def self.setup(config) # Log directly through SemanticLogger, NOT through Shrine.logger # Shrine.logger is a basic Logger that would just call .to_s on the struct ::SemanticLogger[::Shrine].info(log_data) - end) + end # Load the instrumentation plugin without Shrine's default log subscriber, which logs # each event as a line of text (a no-op if the app has loaded it already). Shrine # ignores a subscriber that's already subscribed. ::Shrine.plugin :instrumentation, log_subscriber: nil SHRINE_EVENTS.each do |event_name| - T.unsafe(::Shrine).subscribe(event_name, &shrine_log_subscriber) + ::Shrine.subscribe(event_name, &shrine_log_subscriber) end # The app may load the plugin with the default log subscriber too, before or after this diff --git a/lib/log_struct/monkey_patches/active_support/tagged_logging/formatter.rb b/lib/log_struct/monkey_patches/active_support/tagged_logging/formatter.rb index fda7b4b3..d09b99f4 100644 --- a/lib/log_struct/monkey_patches/active_support/tagged_logging/formatter.rb +++ b/lib/log_struct/monkey_patches/active_support/tagged_logging/formatter.rb @@ -31,14 +31,13 @@ def call(severity, time, progname, data) # Preserve original Rails behavior when LogStruct is disabled return super unless ::LogStruct.enabled? - # Get current tags - tags = T.unsafe(self).respond_to?(:current_tags) ? current_tags : (Thread.current[:activesupport_tagged_logging_tags] || []) + tags = current_tags # Add tags to data as hash entry (not text prefix) data_with_tags = if data.is_a?(Hash) tags.present? ? data.merge(tags: tags) : data elsif data.is_a?(::LogStruct::Log::Interfaces::CommonFields) || (data.is_a?(T::Struct) && data.respond_to?(:serialize)) - hash = T.unsafe(data).serialize + hash = data.serialize tags.present? ? hash.merge(tags: tags) : hash else base = {LogStruct::LogField::Message.serialize => data.to_s} diff --git a/lib/log_struct/rails_boot_banner_silencer.rb b/lib/log_struct/rails_boot_banner_silencer.rb index 6554aec0..a33e8230 100644 --- a/lib/log_struct/rails_boot_banner_silencer.rb +++ b/lib/log_struct/rails_boot_banner_silencer.rb @@ -51,6 +51,8 @@ def self.patch_server_command(server_command) module ServerCommandSilencer extend T::Sig + extend T::Helpers + requires_ancestor { Object } sig { params(args: T.untyped, block: T.nilable(T.proc.returns(T.untyped))).returns(T.untyped) } def perform(*args, &block) @@ -105,9 +107,9 @@ def build_rails_banner_line(url) sig { returns(String) } def lookup_executable - return "rails" unless T.unsafe(self).respond_to?(:executable, true) + return "rails" unless respond_to?(:executable, true) - T.cast(T.unsafe(self).send(:executable), String) + T.cast(send(:executable), String) rescue "rails" end diff --git a/lib/log_struct/semantic_logger/formatter.rb b/lib/log_struct/semantic_logger/formatter.rb index a75357a8..5894cae4 100644 --- a/lib/log_struct/semantic_logger/formatter.rb +++ b/lib/log_struct/semantic_logger/formatter.rb @@ -44,7 +44,7 @@ module SemanticLogger # The formatter produces consistent, parseable JSON that includes: # - Standard log fields (timestamp, level, message, logger name) # - LogStruct-specific fields (source, event, context) - # - SemanticLogger metadata (process ID, thread ID, tags) + # - The request ID and the running Sidekiq job's context, from SemanticLogger's named tags # - Application-specific payload data # # This combination provides the performance benefits of SemanticLogger with diff --git a/lib/log_struct/semantic_logger/logger.rb b/lib/log_struct/semantic_logger/logger.rb index d1518882..c1ea645b 100644 --- a/lib/log_struct/semantic_logger/logger.rb +++ b/lib/log_struct/semantic_logger/logger.rb @@ -107,11 +107,11 @@ def stop_broadcasting_to(logger) # Support for tagged logging sig { params(tags: T.untyped, block: T.proc.returns(T.untyped)).returns(T.untyped) } def tagged(*tags, &block) - # Convert tags to array and pass individually to avoid splat issues tag_array = tags.flatten if tag_array.empty? super(&block) else + # Sorbet can't check a splat of an Array whose size it doesn't know super(*T.unsafe(tag_array), &block) end end diff --git a/lib/log_struct/semantic_logger/setup.rb b/lib/log_struct/semantic_logger/setup.rb index 07129f2a..3c0d737c 100644 --- a/lib/log_struct/semantic_logger/setup.rb +++ b/lib/log_struct/semantic_logger/setup.rb @@ -259,7 +259,7 @@ def self.replace_rails_logger(app) # initializers run, e.g. ActiveRecord would copy an empty filter_parameters into # filter_attributes. COMPONENT_LOAD_HOOKS.each do |hook| - ActiveSupport.on_load(hook) { T.unsafe(self).logger = logger } + ActiveSupport.on_load(hook) { self.logger = logger } end # Store reference in app config @@ -282,7 +282,7 @@ def self.restore_rails_logger(app) # components that are already loaded COMPONENT_LOAD_HOOKS.each do |hook| ActiveSupport.on_load(hook) do - T.unsafe(self).logger = original_logger if T.unsafe(self).logger.equal?(logger) + self.logger = original_logger if self.logger.equal?(logger) end end diff --git a/rails_test_app/templates/app/controllers/logging_controller.rb b/rails_test_app/templates/app/controllers/logging_controller.rb index 9414fa8b..67971862 100644 --- a/rails_test_app/templates/app/controllers/logging_controller.rb +++ b/rails_test_app/templates/app/controllers/logging_controller.rb @@ -108,11 +108,11 @@ def test_job def test_context # TODO: Fix types for the tagged method # Test Rails' built-in tagged logging - T.unsafe(Rails.logger).tagged("REQUEST_ID_123", "USER_456") do + Rails.logger.tagged("REQUEST_ID_123", "USER_456") do Rails.logger.info("Message with tags") # Nested tags - T.unsafe(Rails.logger).tagged("NESTED") do + Rails.logger.tagged("NESTED") do Rails.logger.warn("Message with nested tags") end end diff --git a/sorbet/rbi/overrides/active_support.rbi b/sorbet/rbi/overrides/active_support.rbi index 292d597e..3dc8ffce 100644 --- a/sorbet/rbi/overrides/active_support.rbi +++ b/sorbet/rbi/overrides/active_support.rbi @@ -39,3 +39,10 @@ module ActiveSupport end end end + +module ActiveSupport::LazyLoadHooks + # The block runs with self set to the class that loaded (ActionController::Base, + # ActiveRecord::Base, ...), so Sorbet can't type calls on self inside it + sig { params(name: Symbol, options: T::Hash[Symbol, T.untyped], block: T.proc.bind(T.untyped).void).void } + def on_load(name, options = {}, &block); end +end diff --git a/sorbet/rbi/shims/shrine.rbi b/sorbet/rbi/shims/shrine.rbi index 36a30ae0..2a6dcea2 100644 --- a/sorbet/rbi/shims/shrine.rbi +++ b/sorbet/rbi/shims/shrine.rbi @@ -6,3 +6,9 @@ module Shrine::Plugins::Instrumentation def self.call(event); end end end + +class Shrine + # Added by the instrumentation plugin, which Shrine loads on demand + sig { params(event_name: Symbol, subscriber: T.proc.params(event: T.untyped).void).void } + def self.subscribe(event_name, &subscriber); end +end diff --git a/test/log_struct/configuration_test.rb b/test/log_struct/configuration_test.rb index aac4e594..d22ea140 100644 --- a/test/log_struct/configuration_test.rb +++ b/test/log_struct/configuration_test.rb @@ -525,9 +525,9 @@ def test_enabled_for_puma_via_argv_in_production private def with_rails_filter_parameters(filter_params, &block) - mock_config = T.unsafe(Object.new) + mock_config = Object.new mock_config.define_singleton_method(:filter_parameters) { filter_params } - mock_app = T.unsafe(Object.new) + mock_app = Object.new mock_app.define_singleton_method(:config) { mock_config } LogStruct.config.filters.filter_keys = [] diff --git a/test/log_struct/error_handling_test.rb b/test/log_struct/error_handling_test.rb index 84e6241a..d27315b2 100644 --- a/test/log_struct/error_handling_test.rb +++ b/test/log_struct/error_handling_test.rb @@ -41,14 +41,14 @@ def test_handle_exception_raise def test_handle_exception_log LogStruct.config.error_handling_modes.standard_errors = LogStruct::ErrorHandlingMode::Log error = RuntimeError.new("log me") - logged = [] + logged = T.let([], T::Array[LogStruct::Log::Error]) LogStruct.stub(:error, ->(log) { logged << log }) do LogStruct.handle_exception(error, source: LogStruct::Source::App) end assert_equal 1, logged.length - log_entry = T.unsafe(logged).first + log_entry = T.must(logged.first) assert_equal "log me", log_entry.message end diff --git a/test/log_struct/formatter_test.rb b/test/log_struct/formatter_test.rb index b587a9f8..9b2bfd6d 100644 --- a/test/log_struct/formatter_test.rb +++ b/test/log_struct/formatter_test.rb @@ -176,7 +176,6 @@ def test_process_values_truncates_large_arrays end def test_process_values_does_not_process_beyond_limit_for_large_arrays - T.unsafe(nil) exploding_class = Class.new do attr_reader :index @@ -190,7 +189,6 @@ def touch! end end - T.unsafe(nil) counting_formatter = Class.new(Formatter) do attr_reader :touched_indices @@ -215,7 +213,7 @@ def process_values(value, **options) result = counting_formatter.process_values(values) assert_equal 11, result.length - assert_equal((0...10).to_a, T.unsafe(counting_formatter).touched_indices) + assert_equal((0...10).to_a, counting_formatter.touched_indices) end def test_process_values_handles_recursive_structures @@ -466,7 +464,6 @@ def test_prevents_infinite_recursion_when_error_occurs private def create_user_class - T.unsafe(nil) Class.new do include GlobalID::Identification attr_accessor :id diff --git a/test/log_struct/log/sql_test.rb b/test/log_struct/log/sql_test.rb index 6674a32e..eec2deaf 100644 --- a/test/log_struct/log/sql_test.rb +++ b/test/log_struct/log/sql_test.rb @@ -260,6 +260,7 @@ def create_sql_log(**attributes) additional_data: {} } + # Sorbet can only check a keyword splat of a Hash literal LogStruct::Log::SQL.new(**T.unsafe(default_attributes.merge(attributes))) end end diff --git a/test/log_struct/multi_error_reporter_test.rb b/test/log_struct/multi_error_reporter_test.rb index 7b883427..75dddcbf 100644 --- a/test/log_struct/multi_error_reporter_test.rb +++ b/test/log_struct/multi_error_reporter_test.rb @@ -211,7 +211,7 @@ def test_report_error_with_no_service end def test_reporter_accepts_callable - calls = [] + calls = T.let([], T::Array[T::Hash[Symbol, T.untyped]]) custom_reporter = lambda do |error, context, source| calls << { error: error, @@ -230,7 +230,7 @@ def test_reporter_accepts_callable MultiErrorReporter.report_error(@exception, @context) assert_equal 1, calls.size - payload = T.unsafe(calls).first + payload = T.must(calls.first) assert_equal @exception, payload[:error] assert_equal @context, payload[:context] @@ -253,6 +253,7 @@ def test_callable_reporter_supports_lower_arity def test_setting_invalid_reporter_raises error = assert_raises(TypeError) do + # Not a reporter, on purpose MultiErrorReporter.reporter = T.unsafe(Object.new) end diff --git a/test/log_struct/rails_boot_banner_silencer_test.rb b/test/log_struct/rails_boot_banner_silencer_test.rb index 04422c29..cb1150ac 100644 --- a/test/log_struct/rails_boot_banner_silencer_test.rb +++ b/test/log_struct/rails_boot_banner_silencer_test.rb @@ -192,7 +192,7 @@ def with_kernel_require_raising_load_error kernel_singleton = Kernel.singleton_class kernel_module = Kernel - T.unsafe(self).instance_eval do + instance_eval do kernel_module.class_eval do alias_method :__logstruct_original_instance_require, :require define_method(:require) do |*_args| @@ -208,7 +208,7 @@ def with_kernel_require_raising_load_error end yield ensure - T.unsafe(self).instance_eval do + instance_eval do kernel_module.class_eval do remove_method :require alias_method :require, :__logstruct_original_instance_require diff --git a/test/support/filter_by_line.rb b/test/support/filter_by_line.rb index 76264aed..61318ccc 100644 --- a/test/support/filter_by_line.rb +++ b/test/support/filter_by_line.rb @@ -7,17 +7,17 @@ module FilterByLine module ClassMethods extend T::Helpers + requires_ancestor { Module } def runnable_methods methods = super # Get the target line from the environment line = ENV["LINE"]&.to_i - return methods unless line && T.unsafe(self).name.include?("Test") + return methods unless line && name&.include?("Test") methods.select do |method_name| - # Couldn't figure out how to get typechecking to pass for #instance_method - method = T.unsafe(self).instance_method(method_name) + method = instance_method(method_name) source_location = method.source_location # If source location doesn't contain line info, include the method next true unless source_location