diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e560e0..f45e855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ The format is described in [Contributing notes](CONTRIBUTING.md#changelog-entry- ## 1.5.0 +### Added + +* Added GraphQL cops. ([@Anastastasia-B][]) + +### Changed + * **(Breaking)** Support Ruby >= 2.7.0 * Update rubocop to `1.55.1`. ([@Set27]) @@ -280,3 +286,4 @@ The format is described in [Contributing notes](CONTRIBUTING.md#changelog-entry- [@a.branzeanu]: https://github.com/texpert [@nikitasakov]: https://github.com/nikitasakov [@paydaylight]: https://github.com/paydaylight +[@Anastastasia-B]: https://github.com/Anastastasia-B diff --git a/README.md b/README.md index 4ff0daf..7d90ad2 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,16 @@ inherit_gem: - config/rails-locales.yml ``` +### GraphQL config + +To include specific rules for GraphQL, you can add the following config + +```yaml +inherit_gem: + datarockets-style: + - config/graphql.yml +``` + ### Rspec config For Rspec tests, you can add a special rubocop config diff --git a/config/graphql.yml b/config/graphql.yml new file mode 100644 index 0000000..f091d16 --- /dev/null +++ b/config/graphql.yml @@ -0,0 +1,10 @@ +require: rubocop-graphql + +GraphQL/ExtractInputType: + Enabled: false + +GraphQL/ExtractType: + Enabled: false + +GraphQL/ResolverMethodLength: + Max: 10 diff --git a/datarockets-style.gemspec b/datarockets-style.gemspec index 34ea2b3..be586dd 100644 --- a/datarockets-style.gemspec +++ b/datarockets-style.gemspec @@ -33,6 +33,7 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "rubocop", "~> 1.55.1" + spec.add_dependency "rubocop-graphql", "~> 1.1.1" spec.add_dependency "rubocop-rails", "~> 2.20.2" spec.add_dependency "rubocop-rspec", "~> 2.23.0" diff --git a/doc/STYLE_GUIDE_GRAPHQL.md b/doc/STYLE_GUIDE_GRAPHQL.md new file mode 100644 index 0000000..7431c42 --- /dev/null +++ b/doc/STYLE_GUIDE_GRAPHQL.md @@ -0,0 +1,486 @@ +# GraphQL Style Guide + +This style is based on the rules from [RuboCop::GraphQL](https://github.com/DmitryTsepelev/rubocop-graphql) gem with a few differences. + +## Table of contents + +* [Style](#Style) + +## Style + +* + Each argument should have a description. + [[link](#graphql-argument-description)] + +```ruby +# bad +class BanUser < BaseMutation + argument :uuid, ID, required: true +end + +# good +class BanUser < BaseMutation + argument :uuid, ID, required: true, description: "UUID of the user to ban" +end +``` + +* + All argument names should be snake_case. + [[link](#graphql-argument-name)] + +```ruby +# bad +class BanUser < BaseMutation + argument :userId, ID, required: true +end + +# good +class BanUser < BaseMutation + argument :user_id, ID, required: true +end +``` + +* + Avoid duplicate argument definitions. + [[link](#graphql-argument-uniqueness)] + +```ruby +# bad +class BanUser < BaseMutation + argument :user_id, ID, required: true + argument :user_id, ID, required: true +end + +# good +class BanUser < BaseMutation + argument :user_id, ID, required: true +end +``` + +* + All field definitions should be grouped together. + [[link](#graphql-field-definitions)] + +```ruby +# bad +class UserType < BaseType + field :first_name, String, null: true + + def first_name + object.contact_data.first_name + end + + field :last_name, String, null: true + + def last_name + object.contact_data.last_name + end +end + +# good +class UserType < BaseType + field :first_name, String, null: true + field :last_name, String, null: true + + def first_name + object.contact_data.first_name + end + + def last_name + object.contact_data.last_name + end +end +``` + +* + Each field should have a description. + [[link](#graphql-field-description)] + +```ruby +# bad +class UserType < BaseType + field :name, String, null: true +end + +# good +class UserType < BaseType + field :name, String, "Name of the user", null: true +end +``` + +* + Avoid unnecessary resolver methods in cases when :hash_key option can be used. + [[link](#graphql-field-hash-key)] + +```ruby +# bad +class Types::UserType < Types::BaseObject + field :phone, String, null: true + + def phone + object[:home_phone] + end +end + +# good +class Types::UserType < Types::BaseObject + field :phone, String, null: true, hash_key: :home_phone +end +``` + +* + Avoid unnecessary resolver methods in cases when :method option can be used. + [[link](#graphql-field-mathod)] + +```ruby +# bad +class Types::UserType < Types::BaseObject + field :phone, String, null: true + + def phone + object.home_phone + end +end + +# good +class Types::UserType < Types::BaseObject + field :phone, String, null: true, method: :home_phone +end +``` + +* + All field names should be snake_case. + [[link](#graphql-field-name)] + +```ruby +# bad +class UserType < BaseType + field :firstName, String, null: true +end + +# good +class UserType < BaseType + field :first_name, String, null: true +end +``` + +* + Avoid duplicate field definitions. + [[link](#graphql-field-uniqueness)] + +```ruby +# bad +class UserType < BaseType + field :name, String, null: true + field :phone, String, null: true + field :phone, String, null: true do + argument :something, String, required: false + end +end + +# good +class UserType < BaseType + field :name, String, null: true + field :phone, String, null: true do + argument :something, String, required: false + end +end +``` + +* + Types and mutations should have graphql_name configured only if it's different from the default name. + [[link](#graphql-graphql-name)] + +```ruby +# bad +class UserType < BaseType + graphql_name 'User' +end + +# good +class UserType < BaseType + graphql_name 'Viewer' +end +``` + +* + Use max_complexity configuration in schema files. + [[link](#graphql-max-complexity-schema)] + +```ruby +# good +class AppSchema < BaseSchema + max_complexity 42 +end +``` + +* + Use max_depth configuration in schema files. + [[link](#graphql-max-depth-schema)] + +```ruby +# good +class AppSchema < BaseSchema + max_depth 42 +end +``` + +* + Fields with multiple definitions should be grouped together. + [[link](#graphql-multiple-field-definitions)] + +```ruby +# bad +class UserType < BaseType + field :first_name, String, null: true + + def first_name + object.contact_data.first_name + end + field :first_name, Name, null: true +end + +# good +class UserType < BaseType + field :first_name, String, null: true + field :first_name, Name, null: true + + def first_name + object.contact_data.first_name + end +end +``` + +* + Types that implement Node interface should have `.authorized?` check. Such types can be fetched by ID and therefore should have type level check to avoid accidental information exposure. + If `.authorized?` is defined in a parent class, you can add parent to the "SafeBaseClasses" to avoid offenses in children. + This cop also checks the `can_can_action` or `pundit_role` methods that can be used as part of the Ruby GraphQL Pro. + [[link](#graphql-not-authorized-node-type)] + +```ruby +# bad +class UserType < BaseType + implements GraphQL::Types::Relay::Node + + field :uuid, ID, null: false +end + +# good +class UserType < BaseType + implements GraphQL::Types::Relay::Node + + field :uuid, ID, null: false + + def self.authorized?(object, context) + super && object.owner == context[:viewer] + end +end + +# good +class UserType < BaseType + implements GraphQL::Types::Relay::Node + + pundit_role :staff + + field :uuid, ID, null: false +end + +# good +class UserType < BaseType + implements GraphQL::Types::Relay::Node + + can_can_action :staff + + field :uuid, ID, null: false +end +``` + +* + All types (objects, inputs, interfaces, scalars, unions, mutations, subscriptions, and resolvers) should have a description. + [[link](#graphql-object-description)] + +```ruby +# bad +class Types::UserType < Types::BaseObject + ... +end + +# good +class Types::UserType < Types::BaseObject + description "Represents application user" + ... +end +``` + +* + Arguments should be alphabetically sorted within groups. + [[link](#graphql-ordered-arguments)] + +```ruby +# bad +class UpdateProfile < BaseMutation + argument :uuid, ID, required: true + argument :email, String, required: false + argument :name, String, required: false +end + +# good +class UpdateProfile < BaseMutation + argument :email, String, required: false + argument :name, String, required: false +end + +# good +class UpdateProfile < BaseMutation + argument :uuid, ID, required: true + + argument :email, String, required: false + argument :name, String, required: false +end +``` + +* + Fields should be alphabetically sorted within groups. + [[link](#graphql-ordered-fields)] + +```ruby +# bad +class UpdateProfile < BaseMutation + field :phone, String, null: true + field :name, String, null: true +end + +# good +class UpdateProfile < BaseMutation + field :name, String, null: true + field :phone, String, null: true +end + +# good +class UpdateProfile < BaseMutation + field :phone, String, null: true + + field :name, String, null: true +end +``` + +* + The length of a resolver method should not exceed 10 lines. + [[link](#graphql-resolver-method-length)] + +* + Avoid using :camelize option for arguments where it is unnecessary. + [[link](#graphql-unnecessary-argument-camelize)] + +```ruby +# bad +class UserType < BaseType + argument :filter, String, required: false, camelize: false +end + +# good +class UserType < BaseType + argument :filter, String, required: false +end + +# good +class UserType < BaseType + argument :email_filter, String, required: false, camelize: true +end +``` + +* + Avoid defining an unnecessary alias, method, or resolver_method. + [[link](#graphql-unnecessary-field-alias)] + +```ruby +# bad +field :name, String, "Name of the user", null: true, alias: :name +field :name, String, "Name of the user", null: true, method: :name +field :name, String, "Name of the user", null: true, resolver_method: :name +field :name, String, "Name of the user", null: true, hash_key: :name + +# good +field :name, String, "Name of the user", null: true, alias: :real_name +field :name, String, "Name of the user", null: true, method: :real_name +field :name, String, "Name of the user", null: true, resolver_method: :real_name +field :name, String, "Name of the user", null: true, hash_key: :real_name +``` + +* + Avoid using :camelize option for fields where it is unnecessary. + [[link](#graphql-unnecessary-field-camelize)] + +```ruby +# bad +class UserType < BaseType + field :name, "Name of the user", String, null: true, camelize: true +end + +# good +class UserType < BaseType + field :name, String, "Name of the user", null: true +end + +# good +class UserType < BaseType + field :first_name, "Name of the user", String, null: true, camelize: true +end +``` + +* + Arguments should either be listed explicitly or **rest should be in the resolve signature (and similar methods, such as #authorized?). + [[link](#graphql-unused-argument)] + +```ruby +# bad +class SomeResolver < Resolvers::Base + type SomeType, null: false + + argument :arg1, String, required: true + argument :arg2, String, required: true + + def authorized?; end + def resolve(arg1:); end +end + +# bad +class SomeResolver < Resolvers::Base + type SomeType, null: false + + argument :arg1, String, required: true + argument :arg2, String, required: true + + def resolve; end +end + +# good +class SomeResolver < Resolvers::Base + argument :arg1, String, required: true + argument :user_id, String, required: true, loads: Types::UserType + argument :post_id, String, loads: Types::PostType, as: :article + argument :comment_ids, String, loads: Types::CommentType + + def authorized?(arg1:, user:, article:, comments:); end + def resolve(arg1:, user:, article:, comments:); end +end + +# good +class SomeResolver < Resolvers::Base + argument :arg1, String, required: true + argument :user_id, String, required: true, loads: Types::UserType + argument :comment_ids, String, loads: Types::CommentType + + def resolve(arg1:, **rest); end +end + +# good +class SomeResolver < Resolvers::Base + type SomeType, null: false + + argument :arg1, String, required: true + argument :arg2, String, required: true + + def resolve(args); end +end +```