Skip to content

Go: Add support for the gqlgen library#13602

Merged
owen-mc merged 17 commits intogithub:mainfrom
pwntester:ruby/add_gqlgen_support
Jul 15, 2023
Merged

Go: Add support for the gqlgen library#13602
owen-mc merged 17 commits intogithub:mainfrom
pwntester:ruby/add_gqlgen_support

Conversation

@pwntester
Copy link
Contributor

Adds support for the GQLgen library

@pwntester pwntester marked this pull request as ready for review June 28, 2023 13:21
@pwntester pwntester requested a review from a team as a code owner June 28, 2023 13:21
@pwntester pwntester changed the title Add support for the gqlgen library Go: Add support for the gqlgen library Jun 28, 2023
Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution. There's very little to change. Tests are failing because the stub in graph/generated.go uses two spaces rather than a tab (like the rest of the file does). I've indicated how to switch to an inline expectations test in comments. They are much easier to read and edit in future.

Alvaro Muñoz and others added 6 commits July 13, 2023 21:32
Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
…ema.resolvers.go

Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
)

// CreateTodo is the resolver for the createTodo field.
func (r *mutationResolver) CreateTodo(ctx context.Context, input model.NewTodo) (*model.Todo, error) { // $ resolverParameter="definition of input"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@owen-mc Thanks! is there any documentation about the syntax of these tests? what is the $ for? can you add any string instead of definition of input?

Copy link
Contributor Author

@pwntester pwntester Jul 13, 2023

Choose a reason for hiding this comment

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

Ok, seems like every line where Gqlgen::ResolverParameter matches will be tagged with the resolverParameter tag with its toString value assigned to the tag value, so in the inline test we can check for tags with specific values. I guess the $ is just to flag that line as an inline test and not just an ordinary comment? still not sure where the "definition of input" value comes from? is that the toString() of a DataFlow::ParameterNode?

Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation is at the top of InlineExpectationsTest.qll. "definition of input" is defined by value in hasActualResult in go/ql/test/library-tests/semmle/go/frameworks/gqlgen/gqlgen.ql. definition of X is the toString() of a ParameterNode for a parameter named X.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Will use these tests the next time

Copy link
Contributor

Choose a reason for hiding this comment

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

I should also mention InlineFlowTest, which is very easy to use.

Alvaro Muñoz and others added 3 commits July 13, 2023 22:11
Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
@owen-mc
Copy link
Contributor

owen-mc commented Jul 14, 2023

Tests are failing because of formatting again (mostly tabs vs spaces). That's stopping us from running the actual tests. The easiest way to fix it is to go to go/ql/test/library-tests/semmle/go/frameworks/gqlgen, run gofmt -w . and commit the changes it makes.

All the code changes look good. I will merge when tests pass.

@owen-mc owen-mc merged commit 0b8353e into github:main Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants