From 6ae846cbaee7a217e330483aac6c8e22d554c32f Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 30 Sep 2024 14:11:14 +0900 Subject: [PATCH 1/4] Validate optional type parameter default types --- lib/steep/diagnostic/signature.rb | 30 ++++++++++ lib/steep/signature/validator.rb | 39 +++++++++++++ sig/steep/diagnostic/signature.rbs | 18 ++++++ sig/steep/signature/validator.rbs | 7 +++ test/validation_test.rb | 90 ++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+) diff --git a/lib/steep/diagnostic/signature.rb b/lib/steep/diagnostic/signature.rb index 2884a28a4..863832fc4 100644 --- a/lib/steep/diagnostic/signature.rb +++ b/lib/steep/diagnostic/signature.rb @@ -417,6 +417,34 @@ def header_line end end + class TypeParamDefaultReferenceError < Base + attr_reader :type_param + + def initialize(type_param, location:) + super(location: location) + @type_param = type_param + end + + def header_line + "The default type of `#{type_param.name}` cannot depend on optional type parameters" + end + end + + class UnsatisfiableGenericsDefaultType < Base + attr_reader :param_name, :relation + + def initialize(param_name, relation, location:) + super(location: location) + @param_name = param_name + @relation = relation + end + + def header_line + "The default type of `#{param_name}` doesn't satisfy upper bound constarint: #{relation}" + end + end + + def self.from_rbs_error(error, factory:) case error when RBS::ParsingError @@ -515,6 +543,8 @@ def self.from_rbs_error(error, factory:) Diagnostic::Signature::InconsistentClassModuleAliasError.new(decl: error.alias_entry.decl) when RBS::CyclicClassAliasDefinitionError Diagnostic::Signature::CyclicClassAliasDefinitionError.new(decl: error.alias_entry.decl) + when RBS::TypeParamDefaultReferenceError + Diagnostic::Signature::TypeParamDefaultReferenceError.new(error.type_param, location: error.location) else raise error end diff --git a/lib/steep/signature/validator.rb b/lib/steep/signature/validator.rb index 3c47166d4..3243c7d04 100644 --- a/lib/steep/signature/validator.rb +++ b/lib/steep/signature/validator.rb @@ -252,6 +252,39 @@ def validate_definition_type(definition) end end + def validate_type_params(type_name, type_params) + if error_type_params = RBS::AST::TypeParam.validate(type_params) + error_type_params.each do |type_param| + default_type = type_param.default_type or raise + @errors << Diagnostic::Signature::TypeParamDefaultReferenceError.new(type_param, location: default_type.location) + end + end + + upper_bounds = type_params.each.with_object({}) do |param, bounds| #$ Hash[Symbol, AST::Types::t?] + bounds[param.name] = factory.type_opt(param.upper_bound_type) + end + + checker.push_variable_bounds(upper_bounds) do + type_params.each do |type_param| + param = checker.factory.type_param(type_param) + + default_type = param.default_type or next + upper_bound = param.upper_bound or next + + relation = Subtyping::Relation.new(sub_type: default_type, super_type: upper_bound) + result = checker.check(relation, self_type: nil, instance_type: nil, class_type: nil, constraints: Subtyping::Constraints.empty) + + if result.failure? + @errors << Diagnostic::Signature::UnsatisfiableGenericsDefaultType.new( + type_param.name, + relation, + location: (type_param.default_type || raise).location + ) + end + end + end + end + def validate_one_class_decl(name, entry) rescue_validation_errors(name) do Steep.logger.debug { "Validating class definition `#{name}`..." } @@ -419,6 +452,8 @@ def validate_one_class_decl(name, entry) validate_definition_type(definition) end end + + validate_type_params(name, entry.type_params) end end end @@ -480,6 +515,8 @@ def validate_one_interface(name) Steep.logger.tagged "#{name}" do definition = builder.build_interface(name) + validate_type_params(name, definition.type_params_decl) + upper_bounds = definition.type_params_decl.each.with_object({}) do |param, bounds| bounds[param.name] = factory.type_opt(param.upper_bound_type) end @@ -575,6 +612,8 @@ def validate_one_alias(name, entry = env.type_alias_decls[name]) builder.validate_type_name(outer, entry.decl.location&.aref(:name)) end + validate_type_params(name, entry.decl.type_params) + upper_bounds = entry.decl.type_params.each.with_object({}) do |param, bounds| bounds[param.name] = factory.type_opt(param.upper_bound_type) end diff --git a/sig/steep/diagnostic/signature.rbs b/sig/steep/diagnostic/signature.rbs index 73c1bbc59..5913a09dc 100644 --- a/sig/steep/diagnostic/signature.rbs +++ b/sig/steep/diagnostic/signature.rbs @@ -246,6 +246,24 @@ module Steep def header_line: () -> String end + class TypeParamDefaultReferenceError < Base + attr_reader type_param: RBS::AST::TypeParam + + def initialize: (RBS::AST::TypeParam, location: RBS::Location[untyped, untyped]?) -> void + + def header_line: () -> String + end + + class UnsatisfiableGenericsDefaultType < Base + attr_reader param_name: Symbol + + attr_reader relation: Subtyping::Relation[AST::Types::t] + + def initialize: (Symbol param_name, Subtyping::Relation[AST::Types::t] relation, location: RBS::Location[untyped, untyped]?) -> void + + def header_line: () -> String + end + def self.from_rbs_error: (RBS::BaseError error, factory: AST::Types::Factory) -> Base end end diff --git a/sig/steep/signature/validator.rbs b/sig/steep/signature/validator.rbs index 142ade732..b98a2cdb9 100644 --- a/sig/steep/signature/validator.rbs +++ b/sig/steep/signature/validator.rbs @@ -42,6 +42,13 @@ module Steep def validate: () -> void + # Validate type parameters + # + # 1. References from default type to optional type parameter is error + # 2. The default type should satisfy the upper bound constraint + # + def validate_type_params: (RBS::TypeName, Array[RBS::AST::TypeParam]) -> void + def validate_type_application_constraints: (RBS::TypeName type_name, Array[RBS::AST::TypeParam] type_params, Array[RBS::Types::t] type_args, location: RBS::Location[untyped, untyped]?) -> void def validate_type_application: (RBS::Types::t) -> void diff --git a/test/validation_test.rb b/test/validation_test.rb index 6db4a1477..8f5eef3e5 100644 --- a/test/validation_test.rb +++ b/test/validation_test.rb @@ -1176,4 +1176,94 @@ class User end end end + + def test_validate_type__generics_default_ref + with_checker <<~RBS do |checker| + module A[A_A, A_B = A_A, A_C = A_B] + end + + class B[B_A, B_B = B_A, B_C = B_B] + end + + interface _C[C_A, C_B = C_A, C_C = C_B] + end + + type d[D_A, D_B = D_A, D_C = D_B] = untyped + RBS + + Validator.new(checker: checker).tap do |validator| + validator.validate + refute_predicate validator.each_error.to_a, :empty? + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::TypeParamDefaultReferenceError, error + assert_equal "The default type of `A_C` cannot depend on optional type parameters", error.header_line + assert_equal "A_B", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::TypeParamDefaultReferenceError, error + assert_equal "The default type of `B_C` cannot depend on optional type parameters", error.header_line + assert_equal "B_B", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::TypeParamDefaultReferenceError, error + assert_equal "The default type of `C_C` cannot depend on optional type parameters", error.header_line + assert_equal "C_B", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::TypeParamDefaultReferenceError, error + assert_equal "The default type of `D_C` cannot depend on optional type parameters", error.header_line + assert_equal "D_B", error.location.source + end + end + end + end + + def test_validate_type__generics_default_upperbound + with_checker <<~RBS do |checker| + module A[A_A, A_B < String = A_A, A_C < Array[untyped] = Array[A_A]] + end + + class B[B_A, B_B < String = B_A, B_C < Array[untyped] = Array[B_A]] + end + + interface _C[C_A, C_B < String = C_A, C_C < Array[untyped] = Array[C_A]] + end + + type d[D_A, D_B < String = D_A, D_C < Array[untyped] = Array[D_A]] = untyped + RBS + + Validator.new(checker: checker).tap do |validator| + validator.validate + refute_predicate validator.each_error.to_a, :empty? + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::UnsatisfiableGenericsDefaultType, error + assert_equal "The default type of `A_B` doesn't satisfy upper bound constarint: A_A <: ::String", error.header_line + assert_equal "A_A", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::UnsatisfiableGenericsDefaultType, error + assert_equal "The default type of `B_B` doesn't satisfy upper bound constarint: B_A <: ::String", error.header_line + assert_equal "B_A", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::UnsatisfiableGenericsDefaultType, error + assert_equal "The default type of `C_B` doesn't satisfy upper bound constarint: C_A <: ::String", error.header_line + assert_equal "C_A", error.location.source + end + + assert_any!(validator.each_error) do |error| + assert_instance_of Diagnostic::Signature::UnsatisfiableGenericsDefaultType, error + assert_equal "The default type of `D_B` doesn't satisfy upper bound constarint: D_A <: ::String", error.header_line + assert_equal "D_A", error.location.source + end + end + end + end end From afecc6905d7d4753e5b7373f79afaa0d4cd0febe Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 30 Sep 2024 14:40:01 +0900 Subject: [PATCH 2/4] Print the detail of some of the RBS validation errors --- lib/steep.rb | 1 + lib/steep/diagnostic/result_printer2.rb | 48 ++++++++++++++++++++++++ lib/steep/diagnostic/ruby.rb | 45 ---------------------- lib/steep/diagnostic/signature.rb | 30 +++++++++++---- lib/steep/signature/validator.rb | 7 ++-- sample/sig/generics.rbs | 15 ++++++++ sig/steep/diagnostic/result_printer2.rbs | 13 +++++++ sig/steep/diagnostic/ruby.rbs | 6 --- sig/steep/diagnostic/signature.rbs | 24 +++++++++--- 9 files changed, 122 insertions(+), 67 deletions(-) create mode 100644 lib/steep/diagnostic/result_printer2.rb create mode 100644 sample/sig/generics.rbs create mode 100644 sig/steep/diagnostic/result_printer2.rbs diff --git a/lib/steep.rb b/lib/steep.rb index 20d605d5c..0556a6faf 100644 --- a/lib/steep.rb +++ b/lib/steep.rb @@ -75,6 +75,7 @@ require "steep/subtyping/variable_variance" require "steep/diagnostic/helper" +require "steep/diagnostic/result_printer2" require "steep/diagnostic/ruby" require "steep/diagnostic/signature" require "steep/diagnostic/lsp_formatter" diff --git a/lib/steep/diagnostic/result_printer2.rb b/lib/steep/diagnostic/result_printer2.rb new file mode 100644 index 000000000..fa03a84d7 --- /dev/null +++ b/lib/steep/diagnostic/result_printer2.rb @@ -0,0 +1,48 @@ +module Steep + module Diagnostic + module ResultPrinter2 + def result_line(result) + case result + when Subtyping::Result::Failure + case result.error + when Subtyping::Result::Failure::UnknownPairError + nil + when Subtyping::Result::Failure::UnsatisfiedConstraints + "Unsatisfied constraints: #{result.relation}" + when Subtyping::Result::Failure::MethodMissingError + "Method `#{result.error.name}` is missing" + when Subtyping::Result::Failure::BlockMismatchError + "Incomaptible block: #{result.relation}" + when Subtyping::Result::Failure::ParameterMismatchError + if result.relation.params? + "Incompatible arity: #{result.relation.super_type} and #{result.relation.sub_type}" + else + "Incompatible arity: #{result.relation}" + end + when Subtyping::Result::Failure::PolyMethodSubtyping + "Unsupported polymorphic method comparison: #{result.relation}" + when Subtyping::Result::Failure::SelfBindingMismatch + "Incompatible block self type: #{result.relation}" + end + else + result.relation.to_s + end + end + + def detail_lines + lines = StringIO.new.tap do |io| + failure_path = result.failure_path || [] + failure_path.reverse_each.filter_map do |result| + result_line(result) + end.each.with_index(1) do |message, index| + io.puts "#{" " * (index)}#{message}" + end + end.string.chomp + + unless lines.empty? + lines + end + end + end + end +end diff --git a/lib/steep/diagnostic/ruby.rb b/lib/steep/diagnostic/ruby.rb index f8b528451..f68eecdef 100644 --- a/lib/steep/diagnostic/ruby.rb +++ b/lib/steep/diagnostic/ruby.rb @@ -61,51 +61,6 @@ def detail_lines end end - module ResultPrinter2 - def result_line(result) - case result - when Subtyping::Result::Failure - case result.error - when Subtyping::Result::Failure::UnknownPairError - nil - when Subtyping::Result::Failure::UnsatisfiedConstraints - "Unsatisfied constraints: #{result.relation}" - when Subtyping::Result::Failure::MethodMissingError - "Method `#{result.error.name}` is missing" - when Subtyping::Result::Failure::BlockMismatchError - "Incomaptible block: #{result.relation}" - when Subtyping::Result::Failure::ParameterMismatchError - if result.relation.params? - "Incompatible arity: #{result.relation.super_type} and #{result.relation.sub_type}" - else - "Incompatible arity: #{result.relation}" - end - when Subtyping::Result::Failure::PolyMethodSubtyping - "Unsupported polymorphic method comparison: #{result.relation}" - when Subtyping::Result::Failure::SelfBindingMismatch - "Incompatible block self type: #{result.relation}" - end - else - result.relation.to_s - end - end - - def detail_lines - lines = StringIO.new.tap do |io| - failure_path = result.failure_path || [] - failure_path.reverse_each.filter_map do |result| - result_line(result) - end.each.with_index(1) do |message, index| - io.puts "#{" " * (index)}#{message}" - end - end.string.chomp - - unless lines.empty? - lines - end - end - end - class IncompatibleAssignment < Base attr_reader :lhs_type attr_reader :rhs_type diff --git a/lib/steep/diagnostic/signature.rb b/lib/steep/diagnostic/signature.rb index 863832fc4..f8afe19e8 100644 --- a/lib/steep/diagnostic/signature.rb +++ b/lib/steep/diagnostic/signature.rb @@ -105,12 +105,16 @@ class UnsatisfiableTypeApplication < Base attr_reader :type_name attr_reader :type_arg attr_reader :type_param + attr_reader :result - def initialize(type_name:, type_arg:, type_param:, location:) + include ResultPrinter2 + + def initialize(type_name:, type_arg:, type_param:, result:, location:) super(location: location) @type_name = type_name @type_arg = type_arg @type_param = type_param + @result = result end def header_line @@ -248,14 +252,20 @@ def header_line class ModuleSelfTypeError < Base attr_reader :name attr_reader :ancestor - attr_reader :relation + attr_reader :result + + include ResultPrinter2 - def initialize(name:, ancestor:, relation:, location:) + def initialize(name:, ancestor:, result:, location:) super(location: location) @name = name @ancestor = ancestor - @relation = relation + @result = result + end + + def relation + result.relation end def header_line @@ -431,12 +441,18 @@ def header_line end class UnsatisfiableGenericsDefaultType < Base - attr_reader :param_name, :relation + attr_reader :param_name, :result - def initialize(param_name, relation, location:) + include ResultPrinter2 + + def initialize(param_name, result, location:) super(location: location) @param_name = param_name - @relation = relation + @result = result + end + + def relation + result.relation end def header_line diff --git a/lib/steep/signature/validator.rb b/lib/steep/signature/validator.rb index 3243c7d04..af1b08af2 100644 --- a/lib/steep/signature/validator.rb +++ b/lib/steep/signature/validator.rb @@ -104,6 +104,7 @@ def validate_type_application_constraints(type_name, type_params, type_args, loc unchecked: param.unchecked?, default_type: factory.type_opt(param.default_type) ), + result: result, location: location ) end @@ -277,7 +278,7 @@ def validate_type_params(type_name, type_params) if result.failure? @errors << Diagnostic::Signature::UnsatisfiableGenericsDefaultType.new( type_param.name, - relation, + result, location: (type_param.default_type || raise).location ) end @@ -343,7 +344,7 @@ def validate_one_class_decl(name, entry) name: name, location: ancestor.source&.location || raise, ancestor: ancestor, - relation: relation + result: _1 ) end end @@ -438,7 +439,7 @@ def validate_one_class_decl(name, entry) name: name, location: ancestor.source&.location || raise, ancestor: ancestor, - relation: relation + result: _1 ) end end diff --git a/sample/sig/generics.rbs b/sample/sig/generics.rbs new file mode 100644 index 000000000..276f514d7 --- /dev/null +++ b/sample/sig/generics.rbs @@ -0,0 +1,15 @@ +module LocationHelper : _WithLocation + interface _WithLocation + def location_method: () -> String + end + + def hello: () -> void +end + +class StringGeneric[X < Integer, Y < Integer = String] + def location_method: () -> Integer + + include LocationHelper + extend LocationHelper +end + diff --git a/sig/steep/diagnostic/result_printer2.rbs b/sig/steep/diagnostic/result_printer2.rbs new file mode 100644 index 000000000..4fbd80a4e --- /dev/null +++ b/sig/steep/diagnostic/result_printer2.rbs @@ -0,0 +1,13 @@ +module Steep + module Diagnostic + module ResultPrinter2 : _DiagnosticWithResult + interface _DiagnosticWithResult + def result: () -> Subtyping::Result::t + end + + def result_line: (Subtyping::Result::t result) -> String? + + def detail_lines: () -> String? + end + end +end diff --git a/sig/steep/diagnostic/ruby.rbs b/sig/steep/diagnostic/ruby.rbs index 8206d7704..15c7c3fa0 100644 --- a/sig/steep/diagnostic/ruby.rbs +++ b/sig/steep/diagnostic/ruby.rbs @@ -29,12 +29,6 @@ module Steep def detail_lines: () -> String? end - module ResultPrinter2 : _DiagnosticWithResult - def result_line: (Subtyping::Result::t result) -> String? - - def detail_lines: () -> String? - end - class IncompatibleAssignment < Base attr_reader lhs_type: untyped diff --git a/sig/steep/diagnostic/signature.rbs b/sig/steep/diagnostic/signature.rbs index 5913a09dc..d7ef80977 100644 --- a/sig/steep/diagnostic/signature.rbs +++ b/sig/steep/diagnostic/signature.rbs @@ -55,6 +55,7 @@ module Steep def initialize: (name: RBS::TypeName, args: Array[AST::Types::t], params: Array[Symbol], location: location?) -> void def header_line: () -> String + end class UnsatisfiableTypeApplication < Base @@ -64,9 +65,13 @@ module Steep attr_reader type_param: Interface::TypeParam - def initialize: (type_name: RBS::TypeName, type_arg: AST::Types::t, type_param: Interface::TypeParam, location: location?) -> void + attr_reader result: Subtyping::Result::t + + def initialize: (type_name: RBS::TypeName, type_arg: AST::Types::t, type_param: Interface::TypeParam, result: Subtyping::Result::t, location: location?) -> void def header_line: () -> String + + include ResultPrinter2 end class InvalidMethodOverload < Base @@ -148,11 +153,15 @@ module Steep attr_reader ancestor: RBS::Definition::Ancestor::Instance - attr_reader relation: Subtyping::Relation[AST::Types::t] + attr_reader result: Subtyping::Result::t + + def relation: () -> Subtyping::Relation[AST::Types::t] - def initialize: (name: RBS::TypeName, ancestor: RBS::Definition::Ancestor::Instance, relation: Subtyping::Relation[AST::Types::t], location: location?) -> void + def initialize: (name: RBS::TypeName, ancestor: RBS::Definition::Ancestor::Instance, result: Subtyping::Result::t, location: location?) -> void def header_line: () -> String + + include ResultPrinter2 end # The error is raised when a class variable is defined in both `class_name` and `other_class_name` @@ -167,7 +176,6 @@ module Steep def header_line: () -> String end - class InstanceVariableTypeError < Base attr_reader name: Symbol @@ -257,11 +265,15 @@ module Steep class UnsatisfiableGenericsDefaultType < Base attr_reader param_name: Symbol - attr_reader relation: Subtyping::Relation[AST::Types::t] + attr_reader result: Subtyping::Result::t - def initialize: (Symbol param_name, Subtyping::Relation[AST::Types::t] relation, location: RBS::Location[untyped, untyped]?) -> void + def initialize: (Symbol param_name, Subtyping::Result::t result, location: RBS::Location[untyped, untyped]?) -> void def header_line: () -> String + + def relation: () -> Subtyping::Relation[AST::Types::t] + + include ResultPrinter2 end def self.from_rbs_error: (RBS::BaseError error, factory: AST::Types::Factory) -> Base From 5186c29a38ad255efcc7b42ea97a594075c369df Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 30 Sep 2024 17:03:02 +0900 Subject: [PATCH 3/4] Fix type error --- sig/steep/diagnostic/ruby.rbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sig/steep/diagnostic/ruby.rbs b/sig/steep/diagnostic/ruby.rbs index 15c7c3fa0..5db678000 100644 --- a/sig/steep/diagnostic/ruby.rbs +++ b/sig/steep/diagnostic/ruby.rbs @@ -642,7 +642,7 @@ module Steep attr_reader block_pair: [Interface::Block?, Interface::Block?]? - attr_reader result: Subtyping::Result::Base + attr_reader result: Subtyping::Result::t include ResultPrinter2 From b2f0715d9416331ff5bdfd6b709ce4253325a93d Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 30 Sep 2024 17:03:08 +0900 Subject: [PATCH 4/4] Update rbs-gem --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index a6adb41da..5310788e9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,7 +73,7 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.11.1) ffi (~> 1.0) - rbs (3.6.0.pre.2) + rbs (3.6.0.pre.3) logger rdoc (6.7.0) psych (>= 4.0.0)