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

ruleguard/typematch: implement struct type pattern matching #87

Merged
merged 1 commit into from
Oct 17, 2020

Conversation

quasilyte
Copy link
Owner

struct{$*_} - arbitrary struct
struct{$_; $_} - struct of any 2 fields
struct{$x; $x} - struct of 2 fields of type $x
struct{$*_; $x} - struct that ends with $x-typed field
struct{$x; $*_} - struct that starts with $x-typed field
struct{$*_; $x; $*_} - struct that contains $x-typed field

This is not a direct solution for https://twitter.com/dgryski/status/1317245210041012224,
but it makes us get a little closer to it.

There are several interpretations of Type.Contains() and we need
to decide what should be traversed and whatnot (and how deep).

Until we decide on the exact Type.Contains() semantics, struct
type pattern matching can be a temporary solution to this problem.

I'll add analyzer tests later to see whether it really can be
used in the requested context. I would expect that we need
things like Type.Underlying(), etc. to make it work correctly.
But that's a different story.

Signed-off-by: Iskander Sharipov quasilyte@gmail.com

	struct{$*_} - arbitrary struct
	struct{$_; $_} - struct of any 2 fields
	struct{$x; $x} - struct of 2 fields of type $x
	struct{$*_; $x} - struct that ends with $x-typed field
	struct{$x; $*_} - struct that starts with $x-typed field
	struct{$*_; $x; $*_} - struct that contains $x-typed field

This is not a direct solution for https://twitter.com/dgryski/status/1317245210041012224,
but it makes us get a little closer to it.

There are several interpretations of Type.Contains() and we need
to decide what should be traversed and whatnot (and how deep).

Until we decide on the exact Type.Contains() semantics, struct
type pattern matching can be a temporary solution to this problem.

I'll add analyzer tests later to see whether it really can be
used in the requested context. I would expect that we need
things like Type.Underlying(), etc. to make it work correctly.
But that's a different story.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
@quasilyte
Copy link
Owner Author

I'll update the docs when it will be clear that this typematch package feature can be used from rules without any modifications.

if p == nil {
return nil
}
if len(field.Names) != 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably can be moved above, to prevent parsing when we don't have field.Names

if !ok {
return false
}
if !p.matchIdenticalFielder(sub.subs, typ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just return thing func call? :)

@quasilyte quasilyte merged commit 043f687 into master Oct 17, 2020
@quasilyte quasilyte deleted the quasilyte/feature/typematch_structmatch branch October 17, 2020 17:15
@dgryski
Copy link
Contributor

dgryski commented Oct 17, 2020

/cc @robskillington

@quasilyte
Copy link
Owner Author

quasilyte commented Oct 17, 2020

I think we still need something like Type.Contains:

m["x"].Type.Is(`map[time.Time]$_`)

It will match all maps where key type is time.Time, but it won't handle cases where we want a key underlying type to be time.Time. Contains() could be a simple way to get away with inability to apply a nested type filter/constraint.

Or maybe type expression vars should be available in consequent where operands:

m["x"].Type.Is(`map[$key]$_`) && m["key"].Type.Underlying().Is(`time.Time`)

Although m["key"] is not the best candidate as it would yield fluent.Var type, but it probably should be a TypeExpr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants