Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix path lookup when ancestor finds type with same name as current scope #10901

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions spec/compiler/semantic/const_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ describe "Semantic: const" do
)) { int32 }
end

it "types a nested type with same name" do
assert_type(%(
class Foo
class Foo
A = 1
end
end

Foo::Foo::A
)) { int32 }
end

it "creates container module if not exist when using Path" do
assert_type(%(
Foo::Bar = 1
Expand Down Expand Up @@ -130,6 +142,81 @@ describe "Semantic: const" do
") { int32 }
end

it "finds current type before parents (#4086)" do
assert_type(%(
class Foo
class Bar
class Baz < Foo
def self.foo
Baz.new.foo
end

def foo
1
end
end
end

class Baz
end
end

Foo::Bar::Baz.foo
)) { int32 }
end

it "doesn't count parent types as current type" do
assert_type(%(
class Foo
end

class Bar
class Foo
def foo
1
end
end

class Baz < Foo
def self.bar
Foo.new
end
end
end

Bar::Baz.bar.foo
)) { int32 }
end

it "finds current type only for first path item (1)" do
assert_error %(
class Foo
def self.foo
Foo::Foo
end
end

Foo.foo
),
"undefined constant Foo::Foo"
end

it "finds current type only for first path item (2)" do
assert_error %(
class Foo
class Foo
end

def self.foo
Foo::Foo
end
end

Foo.foo
),
"undefined constant Foo::Foo"
end

it "types a global constant reference in method" do
assert_type("
FOO = 2.5
Expand Down
22 changes: 22 additions & 0 deletions spec/compiler/semantic/new_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,26 @@ describe "Semantic: new" do
),
"wrong number of arguments for 'Bar.new' (given 0, expected 1)"
end

it "uses correct receiver for `initialize` in namespaced classes (#4086)" do
assert_type %(
class Foo
class Baz(T)
HertzDevil marked this conversation as resolved.
Show resolved Hide resolved
end

module Bar
class Baz(T) < Foo
def initialize(x)
end

def foo
'a'
end
end
end
end

Foo::Bar::Baz(Int32).new(1).foo
) { char }
end
end
35 changes: 23 additions & 12 deletions src/compiler/crystal/semantic/path_lookup.cr
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ module Crystal
names.each_with_index do |name, i|
# The search must continue in the namespace only for the first path
# item: for subsequent path items only the parents must be looked up
type = type.lookup_path_item(name, lookup_in_namespace: lookup_in_namespace && i == 0, include_private: i == 0 || include_private, location: location)
type = type.lookup_path_item(name, i == 0, lookup_in_namespace && i == 0, i == 0 || include_private, location)
return unless type

# Stop if this is the last name
Expand All @@ -65,7 +65,11 @@ module Crystal
# type's namespace. This parameter is useful because when writing
# `Foo::Bar::Baz`, `Foo` should be searched in enclosing namespaces,
# but `Bar` and `Baz` not.
def lookup_path_item(name : String, lookup_in_namespace, include_private, location) : Type | ASTNode | Nil
#
# If *lookup_self* is `true`, if the type is not found under `self` but has
# the same name as `self`, then `self` is returned. This has higher
# precedence than ancestors and the enclosing namespace.
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location) : Type | ASTNode | Nil
# First search in our types
type = types?.try &.[name]?
if type
Expand All @@ -76,23 +80,30 @@ module Crystal
return type
end

# Try ourself for the first path item, unless we are the top-level
if lookup_self && self != program
if self.is_a?(NamedType) && name == self.name
return self
end
end

# Then try out parents, but don't search in our parents namespace
parents.try &.each do |parent|
match = parent.lookup_path_item(name, lookup_in_namespace: false, include_private: include_private, location: location)
match = parent.lookup_path_item(name, false, false, include_private, location)
return match if match
end

# Try our namespace, unless we are the top-level
if lookup_in_namespace && self != program
return namespace.lookup_path_item(name, lookup_in_namespace, include_private, location)
return namespace.lookup_path_item(name, false, lookup_in_namespace, include_private, location)
end

nil
end
end

class Program
def lookup_path_item(name : String, lookup_in_namespace, include_private, location)
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location)
# Check if there's a private type in location
if location && (original_filename = location.original_filename) &&
(file_module = file_module?(original_filename)) &&
Expand All @@ -105,7 +116,7 @@ module Crystal
end

module GenericType
def lookup_path_item(name : String, lookup_in_namespace, include_private, location)
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location)
# If we are Foo(T) and somebody looks up the type T, we return `nil` because we don't
# know what type T is, and we don't want to continue search in the namespace
if type_vars.includes?(name)
Expand All @@ -116,7 +127,7 @@ module Crystal
end

class GenericInstanceType
def lookup_path_item(name : String, lookup_in_namespace, include_private, location)
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location)
# Check if *name* is a type variable
if type_var = type_vars[name]?
if type_var.is_a?(Var)
Expand All @@ -125,26 +136,26 @@ module Crystal
type_var
end
else
generic_type.lookup_path_item(name, lookup_in_namespace, include_private, location)
generic_type.lookup_path_item(name, lookup_self, lookup_in_namespace, include_private, location)
end
end
end

class UnionType
def lookup_path_item(name : String, lookup_in_namespace, include_private, location)
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location)
# Union type does not currently inherit GenericClassInstanceType,
# so we check if *name* is the only type variable of Union(*T)
if name == "T"
return program.tuple_of(union_types)
end
program.lookup_path_item(name, lookup_in_namespace, include_private, location)
program.lookup_path_item(name, lookup_self, lookup_in_namespace, include_private, location)
end
end

class AliasType
def lookup_path_item(name : String, lookup_in_namespace, include_private, location)
def lookup_path_item(name : String, lookup_self, lookup_in_namespace, include_private, location)
if aliased_type = aliased_type?
aliased_type.lookup_path_item(name, lookup_in_namespace, include_private, location)
aliased_type.lookup_path_item(name, lookup_self, lookup_in_namespace, include_private, location)
else
super
end
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/semantic/top_level_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -1167,7 +1167,7 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor
unless target_type
next_type = base_type
path.names.each do |name|
next_type = base_type.lookup_path_item(name, lookup_in_namespace: false, include_private: true, location: path.location)
next_type = base_type.lookup_path_item(name, lookup_self: false, lookup_in_namespace: false, include_private: true, location: path.location)
if next_type
if next_type.is_a?(ASTNode)
path.raise "expected #{name} to be a type"
Expand Down