From e309e8359274de710d256e13da27a17cdfcfeb20 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 1 Nov 2023 23:57:47 +0800 Subject: [PATCH] Fix private type definitions with namespaced `Path`s --- spec/compiler/semantic/private_spec.cr | 120 ++++++++---------- .../crystal/semantic/top_level_visitor.cr | 59 ++++----- 2 files changed, 79 insertions(+), 100 deletions(-) diff --git a/spec/compiler/semantic/private_spec.cr b/spec/compiler/semantic/private_spec.cr index 4f339370b02f..bc1cb7ab5e62 100644 --- a/spec/compiler/semantic/private_spec.cr +++ b/spec/compiler/semantic/private_spec.cr @@ -318,76 +318,64 @@ describe "Semantic: private" do )) { int32 } end - it "doesn't find private class from outside namespace" do - assert_error %( - class Foo - private class Bar - end - end - - Foo::Bar - ), - "private constant Foo::Bar referenced" - end - - it "doesn't find private module from outside namespace" do - assert_error %( - class Foo - private module Bar - end - end - - Foo::Bar - ), - "private constant Foo::Bar referenced" - end - - it "doesn't find private enum from outside namespace" do - assert_error %( - class Foo - private enum Bar - A - end - end - - Foo::Bar - ), - "private constant Foo::Bar referenced" - end - - it "doesn't find private alias from outside namespace" do - assert_error %( - class Foo - private alias Bar = Int32 - end - - Foo::Bar - ), - "private constant Foo::Bar referenced" - end - - it "doesn't find private lib from outside namespace" do - assert_error %( - class Foo - private lib LibBar + {% for kind, decl in { + "class" => %(class Bar; end), + "module" => %(module Bar; end), + "enum" => %(enum Bar; A; end), + "alias" => %(alias Bar = Int32), + "lib" => %(lib Bar; end), + "constant" => %(Bar = 1), + } %} + it "doesn't find private {{ kind.id }} from outside namespace" do + assert_error <<-CRYSTAL, "private constant Foo::Bar referenced" + module Foo + private {{ decl.id }} end - end - Foo::LibBar - ), - "private constant Foo::LibBar referenced" - end + Foo::Bar + CRYSTAL + end + {% end %} + + {% for kind, decl in { + "class" => %(class Foo::Bar; end), + "module" => %(module Foo::Bar; end), + "enum" => %(enum Foo::Bar; A; end), + "alias" => %(alias Foo::Bar = Int32), + "lib" => %(lib Foo::Bar; end), + "constant" => %(Foo::Bar = 1), + } %} + it "doesn't find private {{ kind.id }} from outside namespace, long name (#8831)" do + assert_error <<-CRYSTAL, "private constant Foo::Bar referenced" + private {{ decl.id }} + + Foo::Bar + CRYSTAL + end - it "doesn't find private constant from outside namespace" do - assert_error %( - class Foo - private Bar = 1 - end + it "doesn't define incorrect type in top-level namespace (#13511)" do + assert_error <<-CRYSTAL, "undefined constant Bar" + private {{ decl.id }} - Foo::Bar - ), - "private constant Foo::Bar referenced" - end + Bar + CRYSTAL + end + {% end %} + + {% for kind, decl in { + "class" => %(class ::Foo; end), + "module" => %(module ::Foo; end), + "enum" => %(enum ::Foo; A; end), + "alias" => %(alias ::Foo = Int32), + "lib" => %(lib ::Foo; end), + "constant" => %(::Foo = 1), + } %} + it "doesn't define private {{ kind.id }} with global type name" do + assert_error <<-CRYSTAL, "can't declare private type in the global namespace" + private {{ decl.id }} + CRYSTAL + end + {% end %} it "finds private type from inside namespace" do assert_type(%( diff --git a/src/compiler/crystal/semantic/top_level_visitor.cr b/src/compiler/crystal/semantic/top_level_visitor.cr index da6d57ed16ab..5d0cbc477260 100644 --- a/src/compiler/crystal/semantic/top_level_visitor.cr +++ b/src/compiler/crystal/semantic/top_level_visitor.cr @@ -802,11 +802,8 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor annotations = read_annotations - scope, name = lookup_type_def_name(target) - if current_type.is_a?(Program) - scope = program.check_private(target) || scope - end - + name = target.names.last + scope = lookup_type_def_scope(target, target) type = scope.types[name]? if type target.raise "already initialized constant #{type}" @@ -1172,7 +1169,9 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor end def lookup_type_def(node : ASTNode) - scope, name = lookup_type_def_name(node) + path = node.name + scope = lookup_type_def_scope(node, path) + name = path.names.last type = scope.types[name]? if type && node.doc type.doc = node.doc @@ -1180,27 +1179,27 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor {scope, name, type} end - def lookup_type_def_name(node : ASTNode) - scope, name = lookup_type_def_name(node.name) - if current_type.is_a?(Program) - scope = program.check_private(node) || scope - end - {scope, name} - end - - def lookup_type_def_name(path : Path) - if path.names.size == 1 && !path.global? - scope = current_type - name = path.names.first - else - path = path.clone - name = path.names.pop - scope = lookup_type_def_name_creating_modules path - end - - scope = check_type_is_type_container(scope, path) + def lookup_type_def_scope(node : ASTNode, path : Path) + scope = + if path.names.size == 1 + if path.global? + if node.visibility.private? + path.raise "can't declare private type in the global namespace; drop the `private` for the top-level namespace, or drop the leading `::` for the file-private namespace" + end + program + else + if current_type.is_a?(Program) + file_module = program.check_private(node) + end + file_module || current_type + end + else + prefix = path.clone + prefix.names.pop + lookup_type_def_name_creating_modules prefix + end - {scope, name} + check_type_is_type_container(scope, path) end def check_type_is_type_container(scope, path) @@ -1239,14 +1238,6 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor target_type.as(NamedType) end - def current_type_scope(node) - scope = current_type - if scope.is_a?(Program) && node.visibility.private? - scope = program.check_private(node) || scope - end - scope - end - # Turns all finished macros into expanded nodes, and # adds them to the program def process_finished_hooks