Merge branch 'ld-apply-graphql-descriptions-rubocop-to-enums' into 'master'

Extend the Graphql/Descriptions cop to cover enum value descriptions

See merge request gitlab-org/gitlab!55132

.rubocop_manual_todo.yml

@@ -10,6 +10,73 @@
 # - guidelines for use found in
 #   https://docs.gitlab.com/ee/development/contributing/style_guides.html#resolving-rubocop-exceptions.
 
+# WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/322903
+Graphql/Descriptions:
+  Exclude:
+    - 'app/graphql/types/access_level_enum.rb'
+    - 'app/graphql/types/admin/analytics/usage_trends/measurement_identifier_enum.rb'
+    - 'app/graphql/types/alert_management/alert_sort_enum.rb'
+    - 'app/graphql/types/alert_management/domain_filter_enum.rb'
+    - 'app/graphql/types/alert_management/integration_type_enum.rb'
+    - 'app/graphql/types/base_enum.rb'
+    - 'app/graphql/types/ci/config/status_enum.rb'
+    - 'app/graphql/types/ci/job_artifact_file_type_enum.rb'
+    - 'app/graphql/types/ci/pipeline_config_source_enum.rb'
+    - 'app/graphql/types/ci/pipeline_status_enum.rb'
+    - 'app/graphql/types/commit_action_mode_enum.rb'
+    - 'app/graphql/types/commit_encoding_enum.rb'
+    - 'app/graphql/types/container_expiration_policy_cadence_enum.rb'
+    - 'app/graphql/types/container_expiration_policy_keep_enum.rb'
+    - 'app/graphql/types/container_expiration_policy_older_than_enum.rb'
+    - 'app/graphql/types/container_repository_sort_enum.rb'
+    - 'app/graphql/types/design_management/design_version_event_enum.rb'
+    - 'app/graphql/types/error_tracking/sentry_error_status_enum.rb'
+    - 'app/graphql/types/issuable_sort_enum.rb'
+    - 'app/graphql/types/issuable_state_enum.rb'
+    - 'app/graphql/types/issue_sort_enum.rb'
+    - 'app/graphql/types/issue_state_event_enum.rb'
+    - 'app/graphql/types/merge_request_sort_enum.rb'
+    - 'app/graphql/types/merge_request_state_enum.rb'
+    - 'app/graphql/types/milestone_state_enum.rb'
+    - 'app/graphql/types/mutation_operation_mode_enum.rb'
+    - 'app/graphql/types/notes/position_type_enum.rb'
+    - 'app/graphql/types/packages/package_type_enum.rb'
+    - 'app/graphql/types/projects/namespace_project_sort_enum.rb'
+    - 'app/graphql/types/release_sort_enum.rb'
+    - 'app/graphql/types/snippets/blob_action_enum.rb'
+    - 'app/graphql/types/snippets/type_enum.rb'
+    - 'app/graphql/types/snippets/visibility_scopes_enum.rb'
+    - 'app/graphql/types/sort_enum.rb'
+    - 'app/graphql/types/todo_action_enum.rb'
+    - 'app/graphql/types/todo_target_enum.rb'
+    - 'app/graphql/types/tree/type_enum.rb'
+    - 'app/graphql/types/user_state_enum.rb'
+    - 'ee/app/graphql/ee/types/issue_sort_enum.rb'
+    - 'ee/app/graphql/ee/types/list_limit_metric_enum.rb'
+    - 'ee/app/graphql/ee/types/todo_target_enum.rb'
+    - 'ee/app/graphql/types/alert_management/payload_alert_field_name_enum.rb'
+    - 'ee/app/graphql/types/alert_management/payload_alert_field_type_enum.rb'
+    - 'ee/app/graphql/types/boards/epic_wildcard_id_enum.rb'
+    - 'ee/app/graphql/types/boards/iteration_wildcard_id_enum.rb'
+    - 'ee/app/graphql/types/dast_site_profile_validation_status_enum.rb'
+    - 'ee/app/graphql/types/dast_site_validation_strategy_enum.rb'
+    - 'ee/app/graphql/types/epic_sort_enum.rb'
+    - 'ee/app/graphql/types/epic_state_enum.rb'
+    - 'ee/app/graphql/types/epic_state_event_enum.rb'
+    - 'ee/app/graphql/types/geo/registry_state_enum.rb'
+    - 'ee/app/graphql/types/health_status_enum.rb'
+    - 'ee/app/graphql/types/iteration_state_enum.rb'
+    - 'ee/app/graphql/types/move_type_enum.rb'
+    - 'ee/app/graphql/types/requirements_management/requirement_state_enum.rb'
+    - 'ee/app/graphql/types/requirements_management/test_report_state_enum.rb'
+    - 'ee/app/graphql/types/security_scanner_type_enum.rb'
+    - 'ee/app/graphql/types/vulnerability/issue_link_type_enum.rb'
+    - 'ee/app/graphql/types/vulnerability_grade_enum.rb'
+    - 'ee/app/graphql/types/vulnerability_report_type_enum.rb'
+    - 'ee/app/graphql/types/vulnerability_severity_enum.rb'
+    - 'ee/app/graphql/types/vulnerability_sort_enum.rb'
+    - 'ee/app/graphql/types/vulnerability_state_enum.rb'
+
 # WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/267606
 FactoryBot/InlineAssociation:
   Exclude:

rubocop/cop/graphql/descriptions.rb

 # frozen_string_literal: true
 
-# This cop checks for missing GraphQL field descriptions.
+# This cop checks for missing GraphQL descriptions and enforces the description style guide:
+# https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#description-style-guide
 #
-# @example
+# @examples
 #
 #   # bad
-#   class AwfulClass
+#   class AwfulType
 #     field :some_field, GraphQL::STRING_TYPE
 #   end
 #
-#   class TerribleClass
+#   class TerribleType
 #     argument :some_argument, GraphQL::STRING_TYPE
 #   end
 #
-#   class UngoodClass
+#   class UngoodType
 #     field :some_argument,
 #       GraphQL::STRING_TYPE,
 #       description: "A description that does not end in a period"
 #   end
 #
+#   class BadEnum
+#     value "some_value"
+#   end
+#
 #   # good
-#   class GreatClass
+#   class GreatType
 #     argument :some_field,
 #       GraphQL::STRING_TYPE,
 #       description: "Well described - a superb description."
@@ -29,6 +34,10 @@
 #       GraphQL::STRING_TYPE,
 #       description: "A thorough and compelling description."
 #   end
+#
+#   class GoodEnum
+#     value "some_value", "Good description."
+#   end
 
 module RuboCop
   module Cop
@@ -37,19 +46,26 @@ module RuboCop
         MSG_NO_DESCRIPTION = 'Please add a `description` property.'
         MSG_NO_PERIOD = '`description` strings must end with a `.`.'
 
-        # ability_field and permission_field set a default description.
-        def_node_matcher :field_or_argument?, <<~PATTERN
-          (send nil? {:field :argument} ...)
+        def_node_matcher :graphql_describable?, <<~PATTERN
+          (send nil? {:field :argument :value} ...)
+        PATTERN
+
+        def_node_matcher :enum?, <<~PATTERN
+          (send nil? :value ...)
         PATTERN
 
-        def_node_matcher :description, <<~PATTERN
+        def_node_matcher :description_kwarg, <<~PATTERN
           (... (hash <(pair (sym :description) $_) ...>))
         PATTERN
 
+        def_node_matcher :enum_style_description, <<~PATTERN
+          (send nil? :value _ $str ...)
+        PATTERN
+
         def on_send(node)
-          return unless field_or_argument?(node)
+          return unless graphql_describable?(node)
 
-          description = description(node)
+          description = locate_description(node)
 
           return add_offense(node, location: :expression, message: MSG_NO_DESCRIPTION) unless description
 
@@ -59,7 +75,7 @@ module RuboCop
         # Autocorrect missing periods at end of description.
         def autocorrect(node)
           lambda do |corrector|
-            description = description(node)
+            description = locate_description(node)
             next unless description
 
             corrector.insert_after(before_end_quote(description), '.')
@@ -68,6 +84,16 @@ module RuboCop
 
         private
 
+        # Fields and arguments define descriptions using a `description` keyword argument.
+        # Enums may define descriptions this way, or as a second `String` param.
+        def locate_description(node)
+          description = description_kwarg(node)
+
+          return description unless description.nil? && enum?(node)
+
+          enum_style_description(node)
+        end
+
         def no_period?(description)
           # Test that the description node is a `:str` (as opposed to
           # a `#copy_field_description` call) before checking.

spec/rubocop/cop/graphql/descriptions_spec.rb

@@ -91,6 +91,50 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
     end
   end
 
+  context 'enum values' do
+    it 'adds an offense when there is no description' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^ Please add a `description` property.
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description does not end in a period' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'bar'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense when description is correct (defined using `description:`)' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'bar.'
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense when description is correct (defined as a second argument)' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', 'bar.', value: 'foo'
+          end
+        end
+      TYPE
+    end
+  end
+
   describe 'autocorrecting descriptions without periods' do
     it 'can autocorrect' do
       expect_offense(<<~TYPE)