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

Attribute type validation with slipped constructor #5849

Open
p6rt opened this issue Dec 2, 2016 · 5 comments
Open

Attribute type validation with slipped constructor #5849

p6rt opened this issue Dec 2, 2016 · 5 comments
Labels
Bug

Comments

@p6rt
Copy link

@p6rt p6rt commented Dec 2, 2016

Migrated from rt.perl.org#130246 (status was 'open')

Searchable as RT130246$

@p6rt

This comment has been minimized.

Copy link
Author

@p6rt p6rt commented Dec 2, 2016

From tim.bollman@live.com

If you create a typed array attribute for a class and then attempt to assign it in the constructor using a hash slip, it fails type validation.

Error message​: Type check failed in assignment to @​!b; expected Str but got Array ($["a", "b", "c"])

use v6;
use Test;

plan 8;
class A {
  has @​.b;
}
class A-validate {
  has Str @​b;
}
my $a;
my @​b = <a b c>;
my %attr = (b => @​b);
lives-ok { $a = A.new(b => @​b) }, 'A Created';
is($a.b, @​b, 'A.b valid');
lives-ok { $a = A.new(|%attr) }, 'A created (slip)';
is($a.b, @​b, 'A.b valid (slip)');
lives-ok { $a = A-validate.new(b => @​b) }, 'A Created (validate)';
is($a.b, @​b, 'A.b valid (validate)');
lives-ok { $a = A-validate.new(|%attr) }, 'A created (validate, slip)'; # fails
is($a.b, @​b, 'A.b valid (validate, slip)');

Thanks,

Tim Bollman

@p6rt

This comment has been minimized.

Copy link
Author

@p6rt p6rt commented Dec 2, 2016

From @zoffixznet

On Fri, 02 Dec 2016 13​:13​:36 -0800, TimTom wrote​:

If you create a typed array attribute for a class and then attempt to
assign it in the constructor using a hash slip, it fails type
validation.

Error message​: Type check failed in assignment to @​!b; expected Str
but got Array ($["a", "b", "c"])

use v6;
use Test;

plan 8;
class A {
has @​.b;
}
class A-validate {
has Str @​b;
}
my $a;
my @​b = <a b c>;
my %attr = (b => @​b);
lives-ok { $a = A.new(b => @​b) }, 'A Created';
is($a.b, @​b, 'A.b valid');
lives-ok { $a = A.new(|%attr) }, 'A created (slip)';
is($a.b, @​b, 'A.b valid (slip)');
lives-ok { $a = A-validate.new(b => @​b) }, 'A Created (validate)';
is($a.b, @​b, 'A.b valid (validate)');
lives-ok { $a = A-validate.new(|%attr) }, 'A created (validate,
slip)'; # fails
is($a.b, @​b, 'A.b valid (validate, slip)');

Thanks,

Tim Bollman

This is the crux of the issue​:

dd [ | (b => []) ]; # [​:b([])]
dd [ |%(b => []) ]; # [​:b($[])]

A pair (slipped or not), does not itemize the array, so the array's values get assigned to the attribute,
but if such a pair has lived in a hash, the array gets itemized, and instead of its values being,
passed into the attribute, the array itself--as a single item--gets passed instead, which in the
second case presented above, triggers the type mismatch.

I don't know whether that's a bug or whether there's anything we can do about this behaviour.

@p6rt

This comment has been minimized.

Copy link
Author

@p6rt p6rt commented Dec 2, 2016

The RT System itself - Status changed from 'new' to 'open'

@p6rt

This comment has been minimized.

Copy link
Author

@p6rt p6rt commented Dec 5, 2016

From tim.bollman@live.com

On Fri, 02 Dec 2016 15​:59​:27 -0800, cpan@​zoffix.com wrote​:

On Fri, 02 Dec 2016 13​:13​:36 -0800, TimTom wrote​:

If you create a typed array attribute for a class and then attempt to
assign it in the constructor using a hash slip, it fails type
validation.

Error message​: Type check failed in assignment to @​!b; expected Str
but got Array ($["a", "b", "c"])

use v6;
use Test;

plan 8;
class A {
has @​.b;
}
class A-validate {
has Str @​b;
}
my $a;
my @​b = <a b c>;
my %attr = (b => @​b);
lives-ok { $a = A.new(b => @​b) }, 'A Created';
is($a.b, @​b, 'A.b valid');
lives-ok { $a = A.new(|%attr) }, 'A created (slip)';
is($a.b, @​b, 'A.b valid (slip)');
lives-ok { $a = A-validate.new(b => @​b) }, 'A Created (validate)';
is($a.b, @​b, 'A.b valid (validate)');
lives-ok { $a = A-validate.new(|%attr) }, 'A created (validate,
slip)'; # fails
is($a.b, @​b, 'A.b valid (validate, slip)');

Thanks,

Tim Bollman

This is the crux of the issue​:

dd [ | (b => []) ]; # [​:b([])]
dd [ |%(b => []) ]; # [​:b($[])]

A pair (slipped or not), does not itemize the array, so the array's
values get assigned to the attribute,
but if such a pair has lived in a hash, the array gets itemized, and
instead of its values being,
passed into the attribute, the array itself--as a single item--gets
passed instead, which in the
second case presented above, triggers the type mismatch.

I don't know whether that's a bug or whether there's anything we can
do about this behaviour.

Ah, I had thought the is($a.b, @​b) would validate that the slipped version was assigning as a set of 3 elements. But if you add a check directly on .elems it returns 1. So at least the assignment is consistently using it as an array of one item, while I thought it was type-checking it one way and assigning as I would expect. I figured that |%args was the way to go because it's the natural way to adjust arguments in a factory constructor like the following. Where you check the arguments for which variant you want to build, adjust parameters accordingly (for example turning string name references into real references), then call the real object constructor.

In particular, it prevents you from doing something like​:

sub build(*%args is copy) {
  %args<b> = %args<b>.map( { perform-magic $_ });
  A.new(|%args)
}

Without the is copy, that passes type validation, except if you pass in something like b => @​b, the assignment will modify the actual elements of @​b which is not ideal in practice. You could instead do something like my @​new-b = ...; A.new(|%args, \(b => @​new-b)); which works as long you can quantify all the parameters you are adjusting, but as things get more complicated, that's not always possible.

Expanded test case attached.

@p6rt

This comment has been minimized.

Copy link
Author

@p6rt p6rt commented Dec 5, 2016

From tim.bollman@live.com

use v6;

use Test;

class A {
  has @​.b;
}

class A-validate {
  has Str @​.b;
}

my @​b = <a b c>;
my %attr = (b => @​b);
my $attr = \(b => @​b);

my %construct = (
  'inline' => { .new(b => @​b) },
  'hash slip' => { .new(|%attr) },
  'capture' => { .new(|$attr) },
  'inner params' => sub ($type) {
  sub do-it(*%args) {
  %args<b> = %args<b>.map( { $_ });
  $type.new(|%args);
  }
  do-it(b => @​b);
  },
  'inner params copy' => sub ($type) {
  sub do-it(*%args is copy) {
  %args<b> = %args<b>.map( { $_ } );
  $type.new(|%args);
  }
  do-it(b => @​b);
  }
);

plan +(A, A-validate) * %construct.elems * 3;

for (A, A-validate) -> $type {
  for %construct.kv -> $version, $construct {
  my $a;
  lives-ok { $a = $construct.($type) }, "Instance Created ({$type.^name} $version)";
  if ($a.defined) {
  is($a.b, @​b, "A.b valid ({$type.^name} $version)");
  is(+$a.b, 3, "A.b length ({$type.^name} $version)");
  }
  }
}

@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.