Skip to content

Commit

Permalink
Replace uses of AliasType#types? by Type#lookup_name (#15068)
Browse files Browse the repository at this point in the history
Introduces `Type#lookup_name` as a dedicated mechanism for looking up a name in a namespace. This allows us to drop the override of `AliasType#types?` which delegated to the aliased type. Thus cyclic hierarchies are no longer possible and we can drop all code (as far as I could find) that was necessary only for handling that.
  • Loading branch information
straight-shoota authored Oct 14, 2024
1 parent d1c5072 commit 4daf486
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 58 deletions.
13 changes: 13 additions & 0 deletions spec/compiler/semantic/did_you_mean_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ describe "Semantic: did you mean" do
"Did you mean 'Foo::Bar'?"
end

it "says did you mean for nested class via alias" do
assert_error <<-CRYSTAL, "Did you mean 'Boo::Bar'?"
class Foo
class Bar
end
end
alias Boo = Foo
Boo::Baz.new
CRYSTAL
end

it "says did you mean finds most similar in def" do
assert_error "
def barbaza
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/crystal/codegen/link.cr
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,6 @@ module Crystal

private def add_link_annotations(types, annotations)
types.try &.each_value do |type|
next if type.is_a?(AliasType) || type.is_a?(TypeDefType)

if type.is_a?(LibType) && type.used? && (link_annotations = type.link_annotations)
annotations.concat link_annotations
end
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/crystal/semantic/abstract_def_checker.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
# ```
class Crystal::AbstractDefChecker
def initialize(@program : Program)
@all_checked = Set(Type).new
end

def run
Expand All @@ -41,9 +40,6 @@ class Crystal::AbstractDefChecker
end

def check_single(type)
return if @all_checked.includes?(type)
@all_checked << type

if type.abstract? || type.module?
type.defs.try &.each_value do |defs_with_metadata|
defs_with_metadata.each do |def_with_metadata|
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/crystal/semantic/new.cr
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ module Crystal
end

def define_default_new(type)
return if type.is_a?(AliasType) || type.is_a?(TypeDefType)

type.types?.try &.each_value do |type|
define_default_new_single(type)
end
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/semantic/path_lookup.cr
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ module Crystal
# 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]?
type = lookup_name(name)
if type
if type.private? && !include_private
return nil
Expand Down
5 changes: 0 additions & 5 deletions src/compiler/crystal/semantic/recursive_struct_checker.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
# Because the type of `Test.@test` would be: `Test | Nil`.
class Crystal::RecursiveStructChecker
@program : Program
@all_checked : Set(Type)

def initialize(@program)
@all_checked = Set(Type).new
end

def run
Expand All @@ -34,9 +32,6 @@ class Crystal::RecursiveStructChecker
end

def check_single(type)
has_not_been_checked = @all_checked.add?(type)
return unless has_not_been_checked

if struct?(type)
target = type
checked = Set(Type).new
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/semantic/suggestions.cr
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ module Crystal
type = self
names.each_with_index do |name, idx|
previous_type = type
type = previous_type.types?.try &.[name]?
type = previous_type.lookup_name(name)
unless type
best_match = Levenshtein.find(name.downcase) do |finder|
previous_type.types?.try &.each_key do |type_name|
previous_type.remove_alias.types?.try &.each_key do |type_name|
finder.test(type_name.downcase, type_name)
end
end
Expand Down
10 changes: 3 additions & 7 deletions src/compiler/crystal/semantic/type_declaration_processor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -621,14 +621,10 @@ struct Crystal::TypeDeclarationProcessor
end

private def remove_duplicate_instance_vars_declarations
# All the types that we checked for duplicate variables
duplicates_checked = Set(Type).new
remove_duplicate_instance_vars_declarations(@program, duplicates_checked)
remove_duplicate_instance_vars_declarations(@program)
end

private def remove_duplicate_instance_vars_declarations(type : Type, duplicates_checked : Set(Type))
return unless duplicates_checked.add?(type)

private def remove_duplicate_instance_vars_declarations(type : Type)
# If a class has an instance variable that already exists in a superclass, remove it.
# Ideally we should process instance variables in a top-down fashion, but it's tricky
# with modules and multiple-inheritance. Removing duplicates at the end is maybe
Expand All @@ -650,7 +646,7 @@ struct Crystal::TypeDeclarationProcessor
end

type.types?.try &.each_value do |nested_type|
remove_duplicate_instance_vars_declarations(nested_type, duplicates_checked)
remove_duplicate_instance_vars_declarations(nested_type)
end
end

Expand Down
7 changes: 0 additions & 7 deletions src/compiler/crystal/tools/doc/generator.cr
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,6 @@ class Crystal::Doc::Generator
def collect_subtypes(parent)
types = [] of Type

# AliasType has defined `types?` to be the types
# of the aliased type, but for docs we don't want
# to list the nested types for aliases.
if parent.is_a?(AliasType)
return types
end

parent.types?.try &.each_value do |type|
case type
when Const, LibType
Expand Down
9 changes: 0 additions & 9 deletions src/compiler/crystal/tools/typed_def_processor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,6 @@ module Crystal::TypedDefProcessor
end

private def process_type(type : Type) : Nil
# Avoid visiting circular hierarchies. There's no use in processing
# alias types anyway.
# For example:
#
# struct Foo
# alias Bar = Foo
# end
return if type.is_a?(AliasType) || type.is_a?(TypeDefType)

if type.is_a?(NamedType) || type.is_a?(Program) || type.is_a?(FileModule)
type.types?.try &.each_value do |inner_type|
process_type inner_type
Expand Down
9 changes: 0 additions & 9 deletions src/compiler/crystal/tools/unreachable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,6 @@ module Crystal
property excludes = [] of String

def process_type(type)
# Avoid visiting circular hierarchies. There's no use in processing
# alias types anyway.
# For example:
#
# struct Foo
# alias Bar = Foo
# end
return if type.is_a?(AliasType) || type.is_a?(TypeDefType)

if type.is_a?(ModuleType)
track_unused_defs type
end
Expand Down
16 changes: 6 additions & 10 deletions src/compiler/crystal/types.cr
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@ module Crystal
nil
end

def lookup_name(name)
types?.try(&.[name]?)
end

def parents
nil
end
Expand Down Expand Up @@ -2756,17 +2760,9 @@ module Crystal
delegate lookup_defs, lookup_defs_with_modules, lookup_first_def,
lookup_macro, lookup_macros, to: aliased_type

def types?
def lookup_name(name)
process_value
if aliased_type = @aliased_type
aliased_type.types?
else
nil
end
end

def types
types?.not_nil!
@aliased_type.try(&.lookup_name(name))
end

def remove_alias
Expand Down

0 comments on commit 4daf486

Please sign in to comment.