From c558bd1e05ba9da237113e7b3f91e045c513e2da Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 20 Dec 2021 15:44:57 +0900 Subject: [PATCH] Implement generic bound validation --- lib/rbs/cli.rb | 34 +++++++++++++++++-- lib/rbs/errors.rb | 13 ++++++++ lib/rbs/location_aux.rb | 2 ++ lib/rbs/validator.rb | 55 ++++++++++++++++++++++++++++++ sig/errors.rbs | 15 +++++++++ sig/location.rbs | 2 ++ sig/validator.rbs | 33 ++++++++++++++++-- test/rbs/cli_test.rb | 74 +++++++++++++++++++++++++++++++++++++++++ test/validator_test.rb | 24 +++++++++++++ 9 files changed, 248 insertions(+), 4 deletions(-) diff --git a/lib/rbs/cli.rb b/lib/rbs/cli.rb index 96fecb17b..7f2f50e15 100644 --- a/lib/rbs/cli.rb +++ b/lib/rbs/cli.rb @@ -430,7 +430,7 @@ def run_validate(args, options) builder = DefinitionBuilder.new(env: env) validator = Validator.new(env: env, resolver: TypeNameResolver.from_env(env)) - env.class_decls.each_key do |name| + env.class_decls.each do |name, decl| stdout.puts "Validating class/module definition: `#{name}`..." builder.build_instance(name).each_type do |type| validator.validate_type type, context: [Namespace.root] @@ -438,13 +438,43 @@ def run_validate(args, options) builder.build_singleton(name).each_type do |type| validator.validate_type type, context: [Namespace.root] end + + d = decl.primary.decl + + validator.validate_type_params( + d.type_params, + type_name: name, + location: d.location&.aref(:type_params) + ) + + decl.decls.each do |d| + d.decl.each_member do |member| + case member + when AST::Members::MethodDefinition + validator.validate_method_definition(member, type_name: name) + end + end + end end - env.interface_decls.each_key do |name| + env.interface_decls.each do |name, decl| stdout.puts "Validating interface: `#{name}`..." builder.build_interface(name).each_type do |type| validator.validate_type type, context: [Namespace.root] end + + validator.validate_type_params( + decl.decl.type_params, + type_name: name, + location: decl.decl.location&.aref(:type_params) + ) + + decl.decl.members.each do |member| + case member + when AST::Members::MethodDefinition + validator.validate_method_definition(member, type_name: name) + end + end end env.constant_decls.each do |name, const| diff --git a/lib/rbs/errors.rb b/lib/rbs/errors.rb index 65ac9d694..246dbe9d9 100644 --- a/lib/rbs/errors.rb +++ b/lib/rbs/errors.rb @@ -443,4 +443,17 @@ def initialize(diagnostic:, location:) super "#{Location.to_string location}: Nonregular generic type alias is prohibited: #{diagnostic.type_name}, #{diagnostic.nonregular_type}" end end + + class CyclicTypeParameterBound < BaseError + attr_reader :params, :type_name, :method_name, :location + + def initialize(type_name:, method_name:, params:, location:) + @type_name = type_name + @method_name = method_name + @params = params + @location = location + + super "#{Location.to_string(location)}: Cyclic type parameter bound is prohibited" + end + end end diff --git a/lib/rbs/location_aux.rb b/lib/rbs/location_aux.rb index 9cb8c48ca..72882ed1c 100644 --- a/lib/rbs/location_aux.rb +++ b/lib/rbs/location_aux.rb @@ -17,6 +17,8 @@ def self.new(buffer_ = nil, start_pos_ = nil, end_pos_ = nil, buffer: nil, start end end + alias aref [] + WithChildren = self def name diff --git a/lib/rbs/validator.rb b/lib/rbs/validator.rb index a1b40041a..47f3e2aee 100644 --- a/lib/rbs/validator.rb +++ b/lib/rbs/validator.rb @@ -80,6 +80,61 @@ def validate_type_alias(entry:) ) end end + + validate_type_params( + entry.decl.type_params, + type_name: type_name, + location: entry.decl.location&.aref(:type_params) + ) + end + end + + def validate_method_definition(method_def, type_name:) + method_def.types.each do |method_type| + unless method_type.type_params.empty? + loc = method_type.location&.aref(:type_params) + + validate_type_params( + method_type.type_params, + type_name: type_name, + method_name: method_def.name, + location: loc + ) + end + end + end + + def validate_type_params(params, type_name: , method_name: nil, location:) + # @type var each_node: TSort::_EachNode[Symbol] + each_node = __skip__ = -> (&block) do + params.each do |param| + block[param.name] + end + end + # @type var each_child: TSort::_EachChild[Symbol] + each_child = __skip__ = -> (name, &block) do + if param = params.find {|p| p.name == name } + if b = param.upper_bound + b.free_variables.each do |tv| + block[tv] + end + end + end + end + + TSort.each_strongly_connected_component(each_node, each_child) do |names| + if names.size > 1 + params = names.map do |name| + params.find {|param| param.name == name} or raise + end + + raise CyclicTypeParameterBound.new( + type_name: type_name, + method_name: method_name, + params: params, + location: location + ) + end end end diff --git a/sig/errors.rbs b/sig/errors.rbs index fef105c0b..7a99f34ee 100644 --- a/sig/errors.rbs +++ b/sig/errors.rbs @@ -230,4 +230,19 @@ module RBS def initialize: (diagnostic: TypeAliasRegularity::Diagnostic, location: Location[untyped, untyped]?) -> void end + + class CyclicTypeParameterBound < BaseError + attr_reader location: Location[untyped, untyped]? + + # Array of parameters which contains cyclic dependencies. + attr_reader params: Array[AST::TypeParam] + + # Type name + attr_reader type_name: TypeName + + # Method name + attr_reader method_name: Symbol? + + def initialize: (type_name: TypeName, method_name: Symbol?, params: Array[AST::TypeParam], location: Location[untyped, untyped]?) -> void + end end diff --git a/sig/location.rbs b/sig/location.rbs index 9ca848fcd..850a54c70 100644 --- a/sig/location.rbs +++ b/sig/location.rbs @@ -79,6 +79,8 @@ module RBS | (OptionalChildKeys) -> Location[bot, bot]? | (Symbol) -> Location[bot, bot]? + alias aref [] + def each_optional_key: () { (Symbol) -> void } -> void | () -> Enumerator[Symbol, void] diff --git a/sig/validator.rbs b/sig/validator.rbs index 4428672cb..0b90e1984 100644 --- a/sig/validator.rbs +++ b/sig/validator.rbs @@ -12,10 +12,39 @@ module RBS def initialize: (env: Environment, resolver: TypeNameResolver) -> void - def absolute_type: (Types::t, context: TypeNameResolver::context) { (Types::t) -> TypeName } -> Types::t - + # Validates the presence of type names and type application arity match. + # def validate_type: (Types::t, context: TypeNameResolver::context) -> void + # Validates type alias definition: + # + # - There is no circular definition between aliases + # - The type alias is _regular_ + # - The generics type parameter variance annotation is consistent with respect to their usage + # - There is no circular dependencies between the generics type parameter bounds + # def validate_type_alias: (entry: Environment::SingleEntry[TypeName, AST::Declarations::Alias]) -> void + + # Validates the type parameters in generic methods. + # + def validate_method_definition: (AST::Members::MethodDefinition, type_name: TypeName) -> void + + # Validates the type parameters if there is no circular dependencies between the bounds. + # + # ```rbs + # [X, Y] # OK + # [X, Y < _Foo[X]] # OK + # [X < _Foo[Y], Y] # OK + # [X < _Foo[Y], Y < _Foo[X]] # Error + # ``` + # + def validate_type_params: (Array[AST::TypeParam] params, type_name: TypeName, ?method_name: Symbol?, location: Location[untyped, untyped]?) -> void + + private + + # Resolves relative type names to absolute type names in given context. + # Yields the type when the type name resolution using `#resolver` fails. + # + def absolute_type: (Types::t, context: TypeNameResolver::context) { (Types::t) -> TypeName } -> Types::t end end diff --git a/test/rbs/cli_test.rb b/test/rbs/cli_test.rb index 56b002cae..a0f2ccf16 100644 --- a/test/rbs/cli_test.rb +++ b/test/rbs/cli_test.rb @@ -202,6 +202,80 @@ module Bar[B] assert_equal "::A", error.type_name.to_s end end + + with_cli do |cli| + Dir.mktmpdir do |dir| + (Pathname(dir) + 'a.rbs').write(<<~RBS) + class Foo[A < _Each[B], B < _Foo[A]] + end + RBS + + error = assert_raises RBS::CyclicTypeParameterBound do + cli.run(["-I", dir, "validate"]) + end + + assert_equal TypeName("::Foo"), error.type_name + assert_nil error.method_name + assert_equal "[A < _Each[B], B < _Foo[A]]", error.location.source + end + end + + with_cli do |cli| + Dir.mktmpdir do |dir| + (Pathname(dir) + 'a.rbs').write(<<~RBS) + class Foo[A < _Each[B]] + def foo: [X < _Foo[Y]] () -> X + + def bar: [X < _Foo[Y], Y < _Bar[Z], Z < _Baz[X]] () -> void + end + RBS + + error = assert_raises RBS::CyclicTypeParameterBound do + cli.run(["-I", dir, "validate"]) + end + + assert_equal TypeName("::Foo"), error.type_name + assert_equal :bar, error.method_name + assert_equal "[X < _Foo[Y], Y < _Bar[Z], Z < _Baz[X]]", error.location.source + end + end + + with_cli do |cli| + Dir.mktmpdir do |dir| + (Pathname(dir) + 'a.rbs').write(<<~RBS) + interface _Foo[A < _Each[B], B < _Baz[A]] + end + RBS + + error = assert_raises RBS::CyclicTypeParameterBound do + cli.run(["-I", dir, "validate"]) + end + + assert_equal TypeName("::_Foo"), error.type_name + assert_nil error.method_name + assert_equal "[A < _Each[B], B < _Baz[A]]", error.location.source + end + end + + with_cli do |cli| + Dir.mktmpdir do |dir| + (Pathname(dir) + 'a.rbs').write(<<~RBS) + interface _Foo[A] + def foo: [X < _Foo[Y]] () -> X + + def bar: [X < _Foo[Y], Y < _Bar[Z], Z < _Baz[X]] () -> void + end + RBS + + error = assert_raises RBS::CyclicTypeParameterBound do + cli.run(["-I", dir, "validate"]) + end + + assert_equal TypeName("::_Foo"), error.type_name + assert_equal :bar, error.method_name + assert_equal "[X < _Foo[Y], Y < _Bar[Z], Z < _Baz[X]]", error.location.source + end + end end def test_constant diff --git a/test/validator_test.rb b/test/validator_test.rb index 1d950324d..1ca7c11ab 100644 --- a/test/validator_test.rb +++ b/test/validator_test.rb @@ -142,4 +142,28 @@ def test_generic_type_aliases end end end + + def test_generic_type_bound + SignatureManager.new do |manager| + manager.add_file("test.rbs", <<-EOF) +type foo[T < String, S < Array[T]] = [T, S] + +type bar[T < _Foo[S], S < _Bar[T]] = nil + EOF + + manager.build do |env| + resolver = RBS::TypeNameResolver.from_env(env) + validator = RBS::Validator.new(env: env, resolver: resolver) + + validator.validate_type_alias(entry: env.alias_decls[type_name("::foo")]) + + error = assert_raises(RBS::CyclicTypeParameterBound) do + validator.validate_type_alias(entry: env.alias_decls[type_name("::bar")]) + end + + assert_equal error.type_name, TypeName("::bar") + assert_equal "[T < _Foo[S], S < _Bar[T]]", error.location.source + end + end + end end