From 4daf48631ed48133f724c4501fa71d0223a4c49a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Mon, 14 Oct 2024 11:10:47 +0200 Subject: [PATCH] Replace uses of `AliasType#types?` by `Type#lookup_name` (#15068) 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. --- spec/compiler/semantic/did_you_mean_spec.cr | 13 +++++++++++++ src/compiler/crystal/codegen/link.cr | 2 -- .../crystal/semantic/abstract_def_checker.cr | 4 ---- src/compiler/crystal/semantic/new.cr | 2 -- src/compiler/crystal/semantic/path_lookup.cr | 2 +- .../crystal/semantic/recursive_struct_checker.cr | 5 ----- src/compiler/crystal/semantic/suggestions.cr | 4 ++-- .../semantic/type_declaration_processor.cr | 10 +++------- src/compiler/crystal/tools/doc/generator.cr | 7 ------- .../crystal/tools/typed_def_processor.cr | 9 --------- src/compiler/crystal/tools/unreachable.cr | 9 --------- src/compiler/crystal/types.cr | 16 ++++++---------- 12 files changed, 25 insertions(+), 58 deletions(-) diff --git a/spec/compiler/semantic/did_you_mean_spec.cr b/spec/compiler/semantic/did_you_mean_spec.cr index cd3f0856ebcb..1c74ebf74c2f 100644 --- a/spec/compiler/semantic/did_you_mean_spec.cr +++ b/spec/compiler/semantic/did_you_mean_spec.cr @@ -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 diff --git a/src/compiler/crystal/codegen/link.cr b/src/compiler/crystal/codegen/link.cr index 81a1a96f4445..33248e93e19b 100644 --- a/src/compiler/crystal/codegen/link.cr +++ b/src/compiler/crystal/codegen/link.cr @@ -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 diff --git a/src/compiler/crystal/semantic/abstract_def_checker.cr b/src/compiler/crystal/semantic/abstract_def_checker.cr index 2a7ccdc05d2a..6d1aa58447a1 100644 --- a/src/compiler/crystal/semantic/abstract_def_checker.cr +++ b/src/compiler/crystal/semantic/abstract_def_checker.cr @@ -24,7 +24,6 @@ # ``` class Crystal::AbstractDefChecker def initialize(@program : Program) - @all_checked = Set(Type).new end def run @@ -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| diff --git a/src/compiler/crystal/semantic/new.cr b/src/compiler/crystal/semantic/new.cr index de8ae55312a0..43a0a631e2c6 100644 --- a/src/compiler/crystal/semantic/new.cr +++ b/src/compiler/crystal/semantic/new.cr @@ -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 diff --git a/src/compiler/crystal/semantic/path_lookup.cr b/src/compiler/crystal/semantic/path_lookup.cr index b2d66879d253..72cab053984b 100644 --- a/src/compiler/crystal/semantic/path_lookup.cr +++ b/src/compiler/crystal/semantic/path_lookup.cr @@ -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 diff --git a/src/compiler/crystal/semantic/recursive_struct_checker.cr b/src/compiler/crystal/semantic/recursive_struct_checker.cr index e7f64913789f..888730e342bb 100644 --- a/src/compiler/crystal/semantic/recursive_struct_checker.cr +++ b/src/compiler/crystal/semantic/recursive_struct_checker.cr @@ -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 @@ -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 diff --git a/src/compiler/crystal/semantic/suggestions.cr b/src/compiler/crystal/semantic/suggestions.cr index 8f4a69d963bc..e9e05612007f 100644 --- a/src/compiler/crystal/semantic/suggestions.cr +++ b/src/compiler/crystal/semantic/suggestions.cr @@ -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 diff --git a/src/compiler/crystal/semantic/type_declaration_processor.cr b/src/compiler/crystal/semantic/type_declaration_processor.cr index 65451741fac3..0e6008b2fa78 100644 --- a/src/compiler/crystal/semantic/type_declaration_processor.cr +++ b/src/compiler/crystal/semantic/type_declaration_processor.cr @@ -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 @@ -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 diff --git a/src/compiler/crystal/tools/doc/generator.cr b/src/compiler/crystal/tools/doc/generator.cr index a2f4db47dee0..4c5988cccae5 100644 --- a/src/compiler/crystal/tools/doc/generator.cr +++ b/src/compiler/crystal/tools/doc/generator.cr @@ -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 diff --git a/src/compiler/crystal/tools/typed_def_processor.cr b/src/compiler/crystal/tools/typed_def_processor.cr index a0a911a6a618..2ba2441d7902 100644 --- a/src/compiler/crystal/tools/typed_def_processor.cr +++ b/src/compiler/crystal/tools/typed_def_processor.cr @@ -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 diff --git a/src/compiler/crystal/tools/unreachable.cr b/src/compiler/crystal/tools/unreachable.cr index 8455a4186882..4ba681240385 100644 --- a/src/compiler/crystal/tools/unreachable.cr +++ b/src/compiler/crystal/tools/unreachable.cr @@ -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 diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr index 5d903b763050..053f6c9d8ac3 100644 --- a/src/compiler/crystal/types.cr +++ b/src/compiler/crystal/types.cr @@ -373,6 +373,10 @@ module Crystal nil end + def lookup_name(name) + types?.try(&.[name]?) + end + def parents nil end @@ -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