Enforce protected beyond top-level namespace

spec/compiler/semantic/visibility_modifiers_spec.cr

@@ -385,6 +385,117 @@ describe "Visibility modifiers" do
       CRYSTAL
   end
 
+  it "disallows invoking protected method from including type's own code (#16827)" do
+    assert_error <<-CRYSTAL, "protected method 'foo' called for NS::Foo"
+      module NS
+        module Mixin
+        end
+
+        class Foo
+          protected def foo
+            1
+          end
+        end
+      end
+
+      class Bar
+        include NS::Mixin
+
+        def bar
+          NS::Foo.new.foo
+        end
+      end
+
+      Bar.new.bar
+      CRYSTAL
+  end
+
+  it "disallows invoking protected method from top-level code when a namespaced module is included (#16827)" do
+    assert_error <<-CRYSTAL, "protected method 'foo' called for NS::Foo"
+      module NS
+        module Mixin
+        end
+
+        class Foo
+          protected def foo
+            1
+          end
+        end
+      end
+
+      include NS::Mixin
+
+      NS::Foo.new.foo
+      CRYSTAL
+  end
+
+  it "disallows invoking protected method from subclass's own code outside the superclass's namespace (#16827)" do
+    assert_error <<-CRYSTAL, "protected method 'foo' called for NS::Foo"
+      module NS
+        class Base
+        end
+
+        class Foo
+          protected def foo
+            1
+          end
+        end
+      end
+
+      class Bar < NS::Base
+        def bar
+          NS::Foo.new.foo
+        end
+      end
+
+      Bar.new.bar
+      CRYSTAL
+  end
+
+  it "allows invoking protected method from def defined in the same namespace and included elsewhere (#16827)" do
+    assert_type(<<-CRYSTAL) { int32 }
+      module NS
+        module Mixin
+          def bar
+            Foo.new.foo
+          end
+        end
+
+        class Foo
+          protected def foo
+            1
+          end
+        end
+      end
+
+      class Bar
+        include NS::Mixin
+      end
+
+      Bar.new.bar
+      CRYSTAL
+  end
+
+  it "allows invoking protected method of extended module" do
+    assert_type(<<-CRYSTAL) { int32 }
+      module ClassMethods
+        protected def build
+          1
+        end
+      end
+
+      class Foo
+        extend ClassMethods
+
+        def self.build
+          super
+        end
+      end
+
+      Foo.build
+      CRYSTAL
+  end
+
   it "gives correct error on unknown call (#2838)" do
     assert_error <<-CRYSTAL, "undefined local variable or method 'foo'"
       private foo

spec/std/spec/context_spec.cr

@@ -34,7 +34,7 @@ describe Spec::ExampleGroup do
       child = Spec::ExampleGroup.new(root, "child", "f.cr", 1, 10, false, nil)
       grand_child = Spec::ExampleGroup.new(child, "grand_child", "f.cr", 2, 9, false, nil)
 
-      grand_child.report(:fail, "oops", "f.cr", 3, nil, nil)
+      grand_child.report_for_spec(:fail, "oops", "f.cr", 3, nil, nil)
 
       root.results_for(:fail).first.description.should eq("child grand_child oops")
     end

spec/std/spec/spec_helper.cr

@@ -1,5 +1,15 @@
 require "../spec_helper"
 
+module Spec
+  class ExampleGroup
+    # Exposes the protected `#report` to the specs, which are outside the
+    # `Spec` namespace
+    def report_for_spec(status : Status, description, file, line, elapsed = nil, ex = nil)
+      report(status, description, file, line, elapsed, ex)
+    end
+  end
+end
+
 class FakeRootContext < Spec::RootContext
   def initialize
     super(Spec::CLI.new)

spec/support/mt_abort_timeout.cr

@@ -2,6 +2,17 @@
 
 private SPEC_TIMEOUT = 15.seconds
 
+module Spec
+  class Example
+    # Defined inside the `Spec` namespace to have access to the protected
+    # `ExampleGroup#report`
+    def report_timeout(timeout)
+      ex = Spec::AssertionFailed.new("spec timed out after #{timeout}", file, line)
+      parent.report(:fail, description, file, line, timeout, ex)
+    end
+  end
+end
+
 Spec.around_each do |example|
   done = Channel(Exception?).new
 
@@ -24,8 +35,6 @@ Spec.around_each do |example|
   when res = done.receive
     raise res if res
   when timeout(timeout)
-    _it = example.example
-    ex = Spec::AssertionFailed.new("spec timed out after #{timeout}", _it.file, _it.line)
-    _it.parent.report(:fail, _it.description, _it.file, _it.line, timeout, ex)
+    example.example.report_timeout(timeout)
   end
 end

src/compiler/crystal/semantic/call_error.cr

@@ -971,9 +971,23 @@ class Crystal::Call
       scope_type = scope.instance_type
       owner_type = match.def.owner.instance_type
 
-      unless scope_type.has_protected_access_to?(owner_type)
-        raise "protected method '#{match.def.name}' called for #{match.def.owner}"
-      end
+      return if scope_type.has_protected_access_to?(owner_type)
+
+      # Check the metaclasses too: a class method gained by `extend`ing a
+      # module belongs to the metaclass hierarchy, which the instance
+      # types above don't reflect
+      return if scope.has_protected_access_to?(match.def.owner, allow_same_namespace: false)
+
+      # The namespace that grants protected access is the one where the
+      # calling code is written. A method inherited from a parent type or an
+      # included module keeps the access granted by the namespace it's
+      # defined in, but including a module doesn't grant the including
+      # type's own code access to protected methods of other types in the
+      # module's namespace (#16827)
+      defining_type = parent_visitor.path_lookup.try(&.instance_type.devirtualize)
+      return if defining_type && defining_type.same_namespace?(owner_type)
+
+      raise "protected method '#{match.def.name}' called for #{match.def.owner}"
     when .public?
       # okay
     end

src/compiler/crystal/types.cr

@@ -338,8 +338,7 @@ module Crystal
 
     # Returns true if `self` and *other* are in the same namespace.
     def same_namespace?(other)
-      top_namespace(self) == top_namespace(other) ||
-        !!parents.try &.any?(&.same_namespace?(other))
+      top_namespace(self) == top_namespace(other)
     end
 
     private def top_namespace(type)