Phase 3a: I/O contracts — documentation and phase record

.rubocop.yml

@@ -172,14 +172,12 @@ Dexpace/NoKeywordSplat:
   Enabled: true
   Include:
     - "gems/*/lib/**/*.rb"
-# Design §9 Addendum A1 (phase 2): a bare Thread/Queue/Mutex/JSON inside these two namespaces
-# rebinds to the adapter gem's constant the moment that gem is required, and core's own suite
-# never requires it.
+# Design §9 Addendum A1 (phase 2), widened by phase 3a's P3-7: a bare Thread/Queue/Mutex/JSON/IO
+# inside module Dexpace resolves to the shadowing Dexpace:: constant -- the adapter gem's once that
+# gem is required, and core's own Dexpace::IO from the first require. Every gem, because an adapter
+# gem also writes inside module Dexpace. test/** is deliberately absent: a test file's classes are
+# top level, so the cop's own lexical check would find nothing there.
 Dexpace/QualifiedCoreConstant:
   Enabled: true
   Include:
-    - "gems/dexpace-core/lib/dexpace/async/**/*.rb"
-    - "gems/dexpace-core/lib/dexpace/serde/**/*.rb"
-    # The seam module itself reopens Dexpace::Serde from one directory up, so a bare JSON there
-    # is the same hazard as one inside serde/.
-    - "gems/dexpace-core/lib/dexpace/serde.rb"
+    - "gems/*/lib/**/*.rb"

.rubocop/cops/dexpace/qualified_core_constant.rb

@@ -4,50 +4,63 @@
 module RuboCop
   module Cop
     module Dexpace
-      # Inside `module Dexpace::Async` and `module Dexpace::Serde`, a bare `Thread`, `Queue`,
-      # `Mutex`, `SizedQueue`, `ConditionVariable` or `JSON` is a silent time bomb: it resolves to
-      # Ruby's class until the adapter gem that defines `Dexpace::Async::Thread` or
-      # `Dexpace::Serde::JSON` is required, and to that module afterwards.
+      # Inside `module Dexpace`, anywhere, a bare `Thread`, `Queue`, `Mutex`, `SizedQueue`,
+      # `ConditionVariable`, `JSON` or `IO` is a silent time bomb: it resolves to Ruby's class
+      # until the `Dexpace::` constant that shadows it is defined, and to that constant
+      # afterwards.
       #
-      # Verified on Ruby 3.2.11 and 4.0.6: before the adapter loads, a bare `Thread` inside
-      # `module Dexpace; module Async` is `Thread`; after, it is `Dexpace::Async::Thread`, and
-      # `Thread.new` raises `NoMethodError: undefined method 'new' for module
-      # Dexpace::Async::Thread`. dexpace-core's own suite never requires that gem, so this is a bug
-      # that cannot fail in the tree that contains it -- which is what a cop is for.
+      # Phase 2 verified the adapter-gem half on 3.2.11 and 4.0.6: a bare `Thread` inside
+      # `module Dexpace; module Async` is `Thread` before `dexpace-async-thread` is required and
+      # `Dexpace::Async::Thread` after, and core's own suite never requires that gem.
       #
-      # Scoped by `Include:` in .rubocop.yml to gems/dexpace-core/lib/dexpace/{async,serde}/**/*.rb.
+      # Phase 3a adds `IO`, and it is worse, because `Dexpace::IO` is defined by CORE. Verified on
+      # 3.2.11, 3.4.10 and 4.0.6: inside `module Dexpace`, `x.is_a?(IO)` and `IO === x` are
+      # silently `false` for a real `::IO`, and a `case/when IO` falls through -- while a bare
+      # `IO.pipe` is a loud NoMethodError. The silent case is exactly the shape design §3.1 reaches
+      # for, and `Response#body_string`, which phase 3b adds under lib/dexpace/http/, is the
+      # counter-example that forbids scoping this rule to lib/dexpace/io/**.
       #
-      # The enclosing namespace is checked here rather than left to `Include:` in .rubocop.yml.
-      # Phase 0's cop harness runs a bare Commissioner over a fixed path with a Config carrying
-      # only TargetRubyVersion, so no `Include:` ever applies to a cop under test -- a cop that
-      # relied on path scoping would flag its own accepted case and the suite would say so.
-      # `Include:` stays in .rubocop.yml anyway, because it is what keeps the cop off the other
-      # five gems during a real run.
+      # The standing rule, for every later phase: a constant joins SHADOWED in the same change
+      # that creates the `Dexpace::` constant which shadows it -- never earlier, never later.
+      # `File`, `StringIO` and `Tempfile` are deliberately absent: no `Dexpace::` constant of those
+      # names exists, a bare `File` inside `module Dexpace` resolves to `::File` (verified), and a
+      # rule guarding nothing only flags correct code.
       #
-      # SHADOWED is one list for both watched namespaces rather than one list each, so a bare
-      # `JSON` inside Dexpace::Async is flagged even though only Dexpace::Serde reopens JSON. Two
-      # per-namespace lists would be a second rule to keep in step for no gain, and the false
-      # positive costs one `::`.
+      # Scoped by `Include:` in .rubocop.yml to gems/*/lib/**/*.rb -- every gem, because an
+      # adapter gem also writes inside `module Dexpace`. test/** is deliberately left out: a test
+      # file's classes are top level, not inside `module Dexpace`, so the lexical check below
+      # would find nothing there.
+      #
+      # The enclosing namespace is checked here rather than left to `Include:`. Phase 0's cop
+      # harness runs a bare Commissioner over a fixed path with a Config carrying only
+      # TargetRubyVersion, so no `Include:` ever applies to a cop under test -- a cop that relied
+      # on path scoping would flag its own accepted case and the suite would say so.
       #
       # @example
-      #   # bad -- inside module Dexpace; module Async
-      #   Queue.new
+      #   # bad -- inside module Dexpace
+      #   IO.pipe
       #
       #   # good
-      #   ::Queue.new
+      #   ::IO.pipe
       #
-      #   # good -- a different namespace; nothing reopens Dexpace::Transport::Thread
-      #   Thread.new
+      #   # good -- a different namespace; nothing named Elsewhere::IO exists
+      #   IO.pipe
       class QualifiedCoreConstant < Base
-        MSG = "Write `::%<name>s` here: a bare `%<name>s` inside %<namespace>s rebinds to the " \
-              "adapter gem's constant once that gem is required."
+        MSG = "Write `::%<name>s` here: a bare `%<name>s` inside %<namespace>s resolves to the " \
+              "shadowing `Dexpace::` constant instead of Ruby's."
 
-        SHADOWED = %w[Thread Queue Mutex SizedQueue ConditionVariable JSON].freeze
+        SHADOWED = %w[Thread Queue Mutex SizedQueue ConditionVariable JSON IO].freeze
 
-        WATCHED = [%w[Dexpace Async], %w[Dexpace Serde]].freeze
+        # One segment, which subsumes phase 2's two: the rule is now "inside `module Dexpace`,
+        # anywhere". Phase 2 deliberately kept ONE SHADOWED list for both namespaces rather than
+        # one list each, and widening the watch rather than adding a per-constant scope keeps that
+        # decision intact. The cost is one `::` on every `::Thread::Mutex` core already writes in
+        # that form.
+        WATCHED = [%w[Dexpace]].freeze
 
         def on_const(node)
-          return if node.namespace # already qualified: `A::Thread` or `::Thread`
+          return if node.namespace # already qualified: `A::IO` or `::IO`
+          return if definition_name?(node)
           return unless SHADOWED.include?(node.short_name.to_s)
 
           namespace = watched_namespace(node)
@@ -58,6 +71,18 @@ def on_const(node)
 
         private
 
+        # `module Dexpace; module IO` DECLARES the shadowing constant; it does not refer to Ruby's.
+        # Without this, the cop flags lib/dexpace/io.rb -- the very file that creates the hazard
+        # the rule exists to guard. Phase 2 never met this because no file declares
+        # `module Dexpace::Async::Thread`.
+        def definition_name?(node)
+          parent = node.parent
+          return false if parent.nil?
+          return false unless parent.module_type? || parent.class_type?
+
+          parent.identifier.equal?(node)
+        end
+
         # @return [String, nil] the watched namespace this node is lexically inside, or nil
         def watched_namespace(node)
           path = lexical_path(node)
@@ -66,7 +91,7 @@ def watched_namespace(node)
         end
 
         # Outermost-first, with a compact `module A::B` split into its segments, so that both
-        # `module Dexpace; module Async` and `module Dexpace::Async` read the same. A
+        # `module Dexpace; module IO` and `module Dexpace::IO` read the same. A
         # `class << self` is an sclass node, not a class node, so it contributes no segment.
         def lexical_path(node)
           node.each_ancestor(:module, :class)

.rubocop/test/cops_test.rb

@@ -5,8 +5,9 @@
 
 # The original five cops and Dexpace/NoKeywordSplat, which between them mechanise CLAUDE.md's
 # ban list and NFR-13. One row per case, one generated Minitest method per row, so a failure
-# names exactly one input. Phase 2's sixth cop has its own tables in the nested class below,
-# because Metrics/ClassLength caps one class at 100 lines and counts a table row as a line.
+# names exactly one input. Phase 2's seventh cop has its own tables in the nested classes below,
+# because Metrics/ClassLength caps one class at 100 lines and counts a table row as a line;
+# phase 3a's extension of it (P3-7) has a second nested class for the same reason.
 class CopsTest < CopCase
   D = RuboCop::Cop::Dexpace
   HEADER = "# frozen_string_literal: true\n# SPDX-License-Identifier: MIT\n\n"
@@ -132,7 +133,7 @@ class CopsTest < CopCase
   # Design §9 Addendum A1 (phase 2): a bare Thread/Queue/Mutex/JSON inside Dexpace::Async or
   # Dexpace::Serde rebinds to the adapter gem's constant the moment that gem is required, and
   # core's own suite never requires it. The compact `module Dexpace::Async` form is the same
-  # lexical path and is flagged too; one SHADOWED list serves both namespaces, so a bare JSON
+  # lexical path and is flagged too; one SHADOWED list serves every namespace, so a bare JSON
   # inside Dexpace::Async is flagged even though only Dexpace::Serde reopens JSON.
   class QualifiedCoreConstantTest < CopCase
     ASYNC = "module Dexpace\n  module Async\n    %s\n  end\nend\n"
@@ -153,14 +154,17 @@ class QualifiedCoreConstantTest < CopCase
       # A class nested inside the namespace is still lexically inside it, and `Thread::Queue`
       # is a bare `Thread` with `Queue` hanging off it.
       [format(NESTED, "Thread::Queue.new"), "Write `::Thread` here"],
+      # Phase 2 accepted this row: nothing reopens Dexpace::Transport::Thread. Phase 3a's
+      # one-segment watch (P3-7) rejects it, because the rule is now "inside module Dexpace,
+      # anywhere" -- a tightening, never a narrowing, and the row moved rather than vanished.
+      ["module Dexpace\n  module Transport\n    Thread.new\n  end\nend\n", "Write `::Thread` here"],
     ].freeze
 
     # The accepted half is what proves the cop does not reject every occurrence of the six names.
     ACCEPTED = [
       format(ASYNC, "::Thread.new"),
       format(ASYNC, "::Queue.new"),
       format(SERDE, "::JSON.generate(value)"),
-      "module Dexpace\n  module Transport\n    Thread.new\n  end\nend\n",
       "Thread.new\n",
       format(ASYNC, "Dexpace::Async::Thread.new"),
       format(NESTED, "::Thread::Queue.new"),
@@ -179,4 +183,63 @@ class QualifiedCoreConstantTest < CopCase
       end
     end
   end
+
+  # Phase 3a (P3-7): SHADOWED gains `IO` and WATCHED becomes the one-segment `Dexpace`, so the rule
+  # is "inside `module Dexpace`, anywhere". Dexpace::IO is defined by CORE, so the hazard is live
+  # from the first require: inside `module Dexpace`, `x.is_a?(IO)` is silently false for a real
+  # ::IO. Its own nested class, for the same 100-line cap phase 2's rows sit under.
+  class QualifiedCoreConstantIOTest < CopCase
+    DEXPACE = "module Dexpace\n  %s\nend\n"
+    IN_IO = "module Dexpace\n  module IO\n    %s\n  end\nend\n"
+    IN_HTTP = "module Dexpace\n  module Http\n    %s\n  end\nend\n"
+
+    # [source, the fragment the message must carry]
+    REJECTED = [
+      [format(DEXPACE, "IO.pipe"), "Write `::IO` here"],
+      [format(IN_IO, "x.is_a?(IO)"), "Write `::IO` here"],
+      [format(IN_HTTP, "x.is_a?(IO)"), "Write `::IO` here"],
+      ["module Dexpace::Async\n  IO.pipe\nend\n", "Write `::IO` here"],
+      # Phase 2's constants, still rejected under the widened one-segment watch.
+      ["module Dexpace\n  module Async\n    Thread.new\n  end\nend\n", "Write `::Thread` here"],
+      ["module Dexpace\n  module Serde\n    JSON.generate(x)\n  end\nend\n", "Write `::JSON` here"],
+      # The widening itself: a bare Mutex or Queue anywhere inside module Dexpace, not only in the
+      # two namespaces phase 2 watched.
+      [format(DEXPACE, "Mutex.new"), "Write `::Mutex` here"],
+      [format(IN_HTTP, "Queue.new"), "Write `::Queue` here"],
+    ].freeze
+
+    # The accepted half is what proves the cop does not reject every occurrence of the name.
+    ACCEPTED = [
+      format(DEXPACE, "::IO.pipe"),
+      format(IN_IO, "x.is_a?(::IO)"),
+      format(IN_HTTP, "::IO.pipe"),
+      "module Dexpace::Async\n  ::IO.pipe\nend\n",
+      # No enclosing module at all.
+      "IO.pipe\n",
+      # An unrelated namespace: nothing named Elsewhere::IO exists.
+      "module Elsewhere\n  IO.pipe\nend\n",
+      # File, StringIO and Tempfile are deliberately NOT in SHADOWED: no Dexpace:: constant of
+      # those names exists, so a rule covering them would only flag correct code.
+      format(DEXPACE, "File.read(path)\n  StringIO.new\n  Tempfile.create"),
+      # Written out in full.
+      format(DEXPACE, "Dexpace::IO::Buffer.new"),
+      # THE definition site. `module Dexpace; module IO` DECLARES the shadowing constant rather
+      # than referring to Ruby's, so lib/dexpace/io.rb -- the file that creates the hazard -- is
+      # not itself an offense. Without these two rows the cop rejects the phase that ships it.
+      "module Dexpace\n  module IO\n    X = 1\n  end\nend\n",
+      "module Dexpace\n  module IO\n    class Buffer\n    end\n  end\nend\n",
+    ].freeze
+
+    REJECTED.each_with_index do |(source, fragment), index|
+      test "rejects case #{index}: #{source.lines.first.strip}" do
+        assert_offense(D::QualifiedCoreConstant, source, fragment)
+      end
+    end
+
+    ACCEPTED.each_with_index do |source, index|
+      test "accepts case #{index}: #{source.lines.first.strip}" do
+        assert_no_offense(D::QualifiedCoreConstant, source)
+      end
+    end
+  end
 end